wharfkit / antelope

Core types, client interfaces, and other tools for working with Antelope-based blockchains.
Other
42 stars 23 forks source link

WA keys are not supported yet #15

Closed amih closed 2 years ago

amih commented 2 years ago

I got this message when trying to create an EOSIO-Signing-Request, from this line of code,

https://github.com/greymass/eosio-core/blob/9f1d6676bc4957b60205ca0d65eac40697aafe74/src/chain/public-key.ts#L92

What needs to be done to add support for WebAuthn keys? I would like to extend this excellent library in a fork and then offer a pull request. Any pointers to the things I need to do to make it work?

Thanks!

jnordberg commented 2 years ago

First step would be to correctly handle decoding and encoding of the PUB_WA PublicKey types, it has some extra data attached that currently is discarded so that's why it can be encoded again.

Further than that I'm not sure, would need to read up on how the signatures are created and verified.

amih commented 2 years ago

If I understand correctly, the extra data attached are:

  1. User presence flag
  2. the rp.id a.k.a. the website domain

https://github.com/EOSIO/eosio-webauthn-example-app/blob/0d037e4cf84b828f25ea52a1291a2f9b4fca2a97/src/server/server.ts#L113

const { Numeric } = require('eosjs');
const x = pubKey.get(-2);
const y = pubKey.get(-3);

const ser = new Serialize.SerialBuffer({textEncoder: new TextEncoder(), textDecoder: new TextDecoder()});
ser.push((y[31] & 1) ? 3 : 2);
ser.pushArray(x);
ser.push(1);                   // <-- enum UserPresence {none = 0,present = 1,verified = 2}
ser.pushString(req.body.rpid); // <-- "website.com"

const compact = ser.asUint8Array();
const key = Numeric.publicKeyToString({
  type: Numeric.KeyType.wa,
  data: compact,
});

I'm still searching for a good place to read on how the signature is created and verified. I tried reading this file but I can't understand it: https://github.com/EOSIO/eosio-webauthn-example-app/blob/0d037e4cf84b828f25ea52a1291a2f9b4fca2a97/src/client/wasig.ts

jnordberg commented 2 years ago

A couple of example public keys (both in binary ABI encoded and PUBWA string format) would be very helpful when implementing it in the serializer as I don't have any way to generate them.

amih commented 2 years ago

You can try interacting with my temporary app at https://eosinabox.amiheines.com You have to d this on a phone, it doesn't work on a laptop. Just tap the "CREATE KEY" button and look at the console log messages.

Here is an example:

pubkey: "PUB_WA_vtARg4AD8k84G61AR2r9bMy5CwnhECSt5xaaPdkoQf5EFmsd9yZvo2DHpr4dZe3R3HxtzE" x: "{\"type\":\"Buffer\",\"data\":[222,200,18,94,64,123,224,9,239,140,164,202,234,87,240,220,20,22,184,202,30,9,101,18,47,7,254,90,27,10,11,128]}" y: "{\"type\":\"Buffer\",\"data\":[132,175,197,35,28,124,33,72,251,45,52,122,251,90,183,207,14,219,9,33,206,239,204,19,205,8,165,87,245,218,206,109]}"

amih commented 2 years ago

I tried playing with this a bit. I'm not sure how to set up the environmet for testing this. Had some typescript errors about the catch error not having type, I added :any to the places were it complained. Probably has to do with typescript newer version being more strict than it used to be.

Anyway, I was tweaking it directly in my node_modules folder, the 1st part is easy, I just commented out this part, so enabling the WA pubkey type.

https://github.com/greymass/eosio-core/blob/9f1d6676bc4957b60205ca0d65eac40697aafe74/src/chain/public-key.ts#L91

The real challenge is in the signing of transactions. That's where @tbfleming can chime in. Here are some telegram messages from him explaining the demo code he wrote here

tbfleming commented 2 years ago

During development, I patched nodeos to allow http://localhost; it normally requires https. This beats mucking around with custom certs. If it'd help I can set up a container with a patched nodeos.

jnordberg commented 2 years ago

I'm trying to wrap my head around the webauthn example code, what's going on here?

https://github.com/EOSIO/eosio-webauthn-example-app/blob/master/src/client/ClientRoot.tsx#L84-L117

Are those some magic constants used by EOSIO or should we be filling in something else there when using in the real world?

Also why is the key data persisted on a server?

tbfleming commented 2 years ago

webauthn requires a server-generated challenge (usually randomly generated). I filled in arbitrary data.

This demo has the server store critical information instead of the client storing it. That way the key doesn't get lost from a cache clear.

jnordberg commented 2 years ago

webauthn requires a server-generated challenge (usually randomly generated). I filled in arbitrary data.

In an EOSIO context is it safe to use a constant there? If not can data be derived from chain state somehow?

This demo has the server store critical information instead of the client storing it. That way the key doesn't get lost from a cache clear.

Hmm, so the EOSIO public key representation doesn't include the key id?

tbfleming commented 2 years ago

In an EOSIO context is it safe to use a constant there? If not can data be derived from chain state somehow?

My initial guess is yes. Someone with a stronger cryptography background (mine is weak) should verify.

Hmm, so the EOSIO public key representation doesn't include the key id?

It includes the rpid (the website origin), but not the credential id.

tbfleming commented 2 years ago

The server is also responsible for storing the relationship between the public key and the credential id.

jnordberg commented 2 years ago

It includes the rpid (the website origin), but not the credential id. The server is also responsible for storing the relationship between the public key and the credential id.

It seems like an oversight it's not included in the public key, if it was you could have just plugged in your key into any browser and entered your account name to sign.

My initial guess is yes. Someone with a stronger cryptography background (mine is weak) should verify.

I think you're right, the challenge is meant only for replay protection according to the webauthn spec

tbfleming commented 2 years ago

Not an oversight. The credential id is borderline sensitive data. We took the safe route at the time. We also didn't want to balloon the public key size any more than we had to.

tbfleming commented 2 years ago

In hindsight, I wish we:

tbfleming commented 2 years ago

The last item opens up a major attack vector, so would require a user to not use the same ubikey for signing transactions and for logging into websites.

tbfleming commented 2 years ago

Made it optional to embed the credential id in the public key

OTOH, this could be stored in a contract

jnordberg commented 2 years ago

I was reading some of the webauthn code in fc and I think it would be possible to allow the key id to be appended to the rpid by having it ignore everything past a # when doing validation keeping the key format and abi the same. Not sure if that would fall under requiring a protocol feature activation or not, if it doesn't it would make for a easy path to enable optional embedding of the key id.

this could be stored in a contract

Yeah I was thinking along the same lines, the RAM cost would be much higher that of just adding it to the key though

tbfleming commented 2 years ago

It would require a protocol feature activation since old versions and new versions would differ on whether a validation passed.

jnordberg commented 2 years ago

Right, that makes sense. At least it can be added later with a protocol feature without breaking backwards compat requiring another key type

On another note, decoding webauthn payloads requires a CBOR library which would increase the bundle size of eosio-core significantly. I'm thinking that the code to go from webauthn payload to core PublicKey/Signature types should live in a separate library with core just being upgraded to pass through the SIG/PUB_WA types.

amih commented 2 years ago

Thank you both @jnordberg and @tbfleming for this!

My understanding is that the server-generated challenge for creating a key is not so important in the blockchain scenario since replay attack is not a concern when the user wants to create a new key pair.

Also, saving the public key and the key id on the client is ok in my case, it is only for the active key. If the user wants, they can back it up however they want - I will provide it as a short json. If they do a cache clear or if they lose access to their phone, they need to initiate a recovery by creating a new key pair and asking their trusted custodian who has the owner key to replace their active public key.

jnordberg commented 2 years ago

https://github.com/greymass/eosio-webauthn

For the helper tools @amih