Add secp256k1 key support for did:key #625

icidasset · 2022-07-25T17:21:47Z

This PR adds secp256k1 key support for did:key, the type of key used by Ethereum, Bitcoin and various other blockchains. We intend to use this initially for wallet auth, where we look up the public key of an Ethereum account and then construct a DID from that client side. In addition to that, we will also sign UCANs with those DIDs, which this PR also adds support for.

Public key types

This PR changes how we store public keys in the database. Before we stored the public key bytes formatted as base64. This is now wrapped in a JSON object, along with the type of the public key. That affects the Update Public Key and Update Public Key via Email Challenge API endpoints for which I created new V3 routes.

New format:

{"key":"Hv+AVRD2WUjUFOsSNbsmrp9fokuwrUnjBcr92f0kxw4=","type":"Ed25519"}
{"key":"AvuqnqcYF49vlzMHKKAuaXjEOaqs/x9V1GnY+rJxx6Y2","type":"Secp256k1"}

Migration

I've written a DB migration that converts the old string format into the json-object format.
This can be found at fission-web-server/sql/1658241548_set_public_key_type.sql
You can run this migration multiple times, it detects both the old and new format.

SECP256K1 UCANs

UCANs with secp256k1 signatures can be created by using the ES256K algorithm (alg field in the header). This alg value is specified here: https://datatracker.ietf.org/doc/html/rfc8812#section-3.2 Not sure that's the correct spec, but it's the only one I found.

Example: https://github.com/fission-codes/webnative-walletauth/blob/375c63881a300442ec3112bf6a7863fc22ecbb4a/src/webnative.ts#L155-L159

Ethereum signatures

This is currently specifically tuned to Ethereum. Normally the content that one would sign for a UCAN signature would be ${UCAN_HEADER}.${UCAN_PAYLOAD}. But here this content will be:

const regularContentBytes =
  uint8arrays.fromString("${UCAN_HEADER}.${UCAN_PAYLOAD}", "utf8")

const actualContent = keccak_256(
  uint8arrays.concat([
    uint8arrays.fromString("\x19Ethereum Signed Message:\n${regularContentBytes.length}", "utf8"),
    regularContentBytes
  ]) 
)

I made a module that is responsible for "wrapping" this data at Web.UCAN.Signature.Secp256k1.Ethereum.

I guess to see if it's an Ethereum signatures we could detect if the recovery value v of the secp256k1 signature is bigger than or equal to 27 (specification: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md#specification). Although I guess Bitcoin has a v value of 27 as well. Thoughts?
Added an Ethereum check which looks for a v value of >= 35.

hs-ucan/library/Crypto/Key/Asymmetric/Public/Types.hs

icidasset · 2022-07-29T11:34:57Z

hs-ucan/library/Crypto/Key/Asymmetric/Public/Types.hs

+fromKeyType "Ed25519"   = fmap Ed25519PublicKey . parseUrlPiece
+fromKeyType "RSA"       = fmap RSAPublicKey . parseUrlPiece
+fromKeyType "Secp256k1" = fmap Secp256k1PublicKey . parseUrlPiece
+fromKeyType _           = \_ -> Left "Invalid Public.Key type"


I tried to do this dynamically with the Data.Data module, but turned into a bit of a hassle, so I kept it simple.

hs-ucan/library/Web/UCAN/Signature/Types.hs

hs-ucan/library/Web/UCAN/Types.hs

icidasset · 2022-07-29T17:15:40Z

@walkah Do I have to do anything special here for Nix and the Github actions? I added a C library https://github.com/bitcoin-core/secp256k1 using Nix (pkgs.secp256k1). Locally I had to add pkgs.pkg-config to get it to work, but I can't get it to work for Github actions. Any thoughts?

Resources:
https://nixos.wiki/wiki/FAQ/I_installed_a_library_but_my_compiler_is_not_finding_it._Why%3F

icidasset · 2022-07-29T17:22:21Z

The error I'm talking about:
https://github.com/fission-codes/fission/runs/7582732721?check_suite_focus=true#step:5:12643

hs-ucan/package.yaml

walkah

I think we may need to apt-get install -y libsecp256k1-dev in the github workflow for the ubuntu native builds, but my suggestion above seems to be working for nixos.

flake.nix

avivash · 2022-08-05T18:21:36Z

hs-ucan/library/Web/UCAN/Validation.hs

+  case publicKey of
+    Secp256k1PublicKey pk ->
+      -- `innerSig` is actually a recoverable signature,
+      -- but since Ethereum (or Metamask?) often has a `v` value


Suggested change

-- but since Ethereum (or Metamask?) often has a `v` value

-- but since Ethereum often has a `v` value

No need to mention Metamask here. Any Ethereum wallet(I would hope) would function in a consistent way 👍🏼 https://eips.ethereum.org/EIPS/eip-155

Thanks, forgot to remove this! Regarding this, see the last paragraph in the PR description, would love your feedback on that 🙏

Thanks, forgot to remove this! Regarding this, see the last paragraph in the PR description, would love your feedback on that 🙏

@icidasset sure! I think EIP-155 may be the key to distinguishing the two. as you mentioned, bitcoin and ethereum used to both have v values of 27(or sometimes 28?), but, looking at EIP-155, it says v = CHAIN_ID * 2 + 35 or v = CHAIN_ID * 2 + 36, so maybe we can just check if v >= 35?

Ah good point, I'll use v >= 35, thanks! Do you know of any other chains that prefix and/or hash the content before signing it?

Ah good point, I'll use v >= 35, thanks! Do you know of any other chains that prefix and/or hash the content before signing it?

hmm 🤔 I believe Tezos and Cosmos prefix them, but I'm not entirely sure how it works across most other chains(I've mostly worked in ethereum). I can do some digging though!

That'd be great! Especially if we want to support other chains than Ethereum in the future 👀

hs-ucan/library/Web/UCAN/Internal/Orphanage/Secp256k1/PublicKey.hs

…56k1/PublicKey.hs Co-authored-by: Andrew Vivash <[email protected]>

icidasset · 2022-08-09T11:00:35Z

Still can't figure out why data-root lookups are returning a 404 for me with my local instance. If anyone else is trying it out. You can test it with walletauth, by putting the following code in the index.ts file (replacing the existing async function):

;(async () => {
  wn.setup.endpoints({
    api: "http://runfission.test:1337",
    lobby: "http://localhost:8001",
    user: "fissionuser.test"
  })

  webnative.login()
})()

avivash · 2022-08-10T19:12:27Z

Still can't figure out why data-root lookups are returning a 404 for me with my local instance. If anyone else is trying it out. You can test it with walletauth, by putting the following code in the index.ts file (replacing the existing async function):
;(async () => {
  wn.setup.endpoints({
    api: "http://runfission.test:1337",
    lobby: "http://localhost:8001",
    user: "fissionuser.test"
  })

  webnative.login()
})()

I don't have a huge amount of context around this, but I just installed nix (i know i'm late to the nix party 😅), got stack setup and i successfully ran stack build on your branch, but nothing seems to be running on port 8001. Is there a specific package I need to run? This is my first time using haskell 🙈

bgins · 2022-08-10T20:11:58Z

I don't have a huge amount of context around this, but I just installed nix (i know i'm late to the nix party 😅), got stack setup and i successfully ran stack build on your branch, but nothing seems to be running on port 8001. Is there a specific package I need to run? This is my first time using haskell 🙈

Hey @avivash! I can give you some pointers on project setup and running the server.

First, a question -- are you using the nix shell in the project? You can use it by running nix-shell at the root of the project. That will take a while to run because it will retrieve and compile a whole lot of Haskell.

Once that finishes and you are in the nix shell, you should be able to run helpme to get a help menu that looks like this:

$ helpme
  ____                                          _     
 / ___|___  _ __ ___  _ __ ___   __ _ _ __   __| |___ 
| |   / _ \| '_ ` _ \| '_ ` _ \ / _` | '_ \ / _` / __|
| |__| (_) | | | | | | | | | | | (_| | | | | (_| \__ \
 \____\___/|_| |_| |_|_| |_| |_|\__,_|_| |_|\__,_|___/
                                                      

build          | Build entire project
cli-install    | Install the Fission CLI
quality        | Run the complete test suite
repl           | Enter the project REPL
server-debug   | Run the Fission Server in debug verbose mode
server-install | Install the Fission Server
server-start   | Run the currently installed Fission Server
server-update  | Update & run the current server to the latest on the current branch
ssh-prod       | SSH into the production environment
ssh-staging    | SSH into the staging environment
watch          | Autobuild with file watcher
welcome        | Print the pretty welcome

The build command is a wrapper around stack build that builds all of the projects (CLI, server, etc). server-install will install the server binary where you can run it, and server-debug will run the server with some logging.

Before you run the server, there's a bit of docker, server and DNS configuration that is documented here: https://github.com/fission-codes/fission/tree/main/fission-web-server#using-docker.

I think that should be enough to get you up and running, but let me know if you'd like a sync chat to help out with any of it.

avivash · 2022-08-10T20:14:23Z

I don't have a huge amount of context around this, but I just installed nix (i know i'm late to the nix party 😅), got stack setup and i successfully ran stack build on your branch, but nothing seems to be running on port 8001. Is there a specific package I need to run? This is my first time using haskell 🙈

Hey @avivash! I can give you some pointers on project setup and running the server.

First, a question -- are you using the nix shell in the project? You can use it by running nix-shell at the root of the project. That will take a while to run because it will retrieve and compile a whole lot of Haskell.

Once that finishes and you are in the nix shell, you should be able to run helpme to get a help menu that looks like this:
$ helpme
  ____                                          _     
 / ___|___  _ __ ___  _ __ ___   __ _ _ __   __| |___ 
| |   / _ \| '_ ` _ \| '_ ` _ \ / _` | '_ \ / _` / __|
| |__| (_) | | | | | | | | | | | (_| | | | | (_| \__ \
 \____\___/|_| |_| |_|_| |_| |_|\__,_|_| |_|\__,_|___/
                                                      

build          | Build entire project
cli-install    | Install the Fission CLI
quality        | Run the complete test suite
repl           | Enter the project REPL
server-debug   | Run the Fission Server in debug verbose mode
server-install | Install the Fission Server
server-start   | Run the currently installed Fission Server
server-update  | Update & run the current server to the latest on the current branch
ssh-prod       | SSH into the production environment
ssh-staging    | SSH into the staging environment
watch          | Autobuild with file watcher
welcome        | Print the pretty welcome
The build command is a wrapper around stack build that builds all of the projects (CLI, server, etc). server-install will install the server binary where you can run it, and server-debug will run the server with some logging.

Before you run the server, there's a bit of docker, server and DNS configuration that is documented here: https://github.com/fission-codes/fission/tree/main/fission-web-server#using-docker.

I think that should be enough to get you up and running, but let me know if you'd like a sync chat to help out with any of it.

Amazing! Thanks @bgins! I'll give that a try 🙏🏼

bgins · 2022-08-10T22:32:50Z

fission-web-server/library/Fission/Web/Server.hs

    v2 =
      genericServerT Fission.RoutesV2
        { api  = serverV2
        , docs = v2Docs
        }

+    serverV3 =
+      genericServerT Fission.V3
+        { user   = genericServerT User.handlerV3


What are your thoughts on versioning the other endpoints like ipfs, app, and auth? Doesn't seem right on this PR, but maybe we will want to version/alias them to have a consistent set of routes?

I don't know, maybe alias them to v1 or v0 sounds like a good idea.

bgins

I'm missing some context on the Ethereum/secp256k1 bits of this PR, but I did a bit of testing and the code looks good to me.

I tested the following:

✅ The database migrates with no issues
✅ Registering a user using walletauth app. A user with an secp256k1 DID registers in the database.
✅ A user's public key can be updated on the update public key v3 route (tested with an Ed25519 PK).

One thing that I tried that failed was updating a user public key from Ed25519 to secp256k1. Not sure if that is a feature that we want.

A question about the old style public keys. Are those a compatibility layer while we migrate the database? Do we keep them around to support v2 routes?

There's more general testing I'd like to do, kicking the tires on other parts of the system overall, but I think that can happen when these changes are on staging.

bgins · 2022-08-10T23:00:35Z

Still can't figure out why data-root lookups are returning a 404 for me with my local instance. If anyone else is trying it out. You can test it with walletauth, by putting the following code in the index.ts file (replacing the existing async function):
;(async () => {
  wn.setup.endpoints({
    api: "http://runfission.test:1337",
    lobby: "http://localhost:8001",
    user: "fissionuser.test"
  })

  webnative.login()
})()

I also ran into these issues, making calls from Insomnia. Seems to be down where we are calling on PowerDNS to resolve DNS records. Do we know if this is an issue in outside of this PR? If so, maybe we could test it on a pizza instance? Or move forward and test this part of the functionality on staging?

walkah · 2022-08-10T23:15:31Z

I also ran into these issues, making calls from Insomnia. Seems to be down where we are calling on PowerDNS to resolve DNS records. Do we know if this is an issue in outside of this PR? If so, maybe we could test it on a pizza instance? Or move forward and test this part of the functionality on staging?

If it's working locally - and tests are all passing - I'd vote to merge and deploy to staging for additional testing (and open subsequent issues if stuff comes up on staging).

icidasset · 2022-08-11T09:45:19Z

@bgins

I tested the following

Awesome thanks!

One thing that I tried that failed was updating a user public key from Ed25519 to secp256k1. Not sure if that is a feature that we want.

Hmm, that should be possible. How did it fail?

A question about the old style public keys. Are those a compatibility layer while we migrate the database? Do we keep them around to support v2 routes?

Indeed, v2 routes still work with the old format. No database migration needed for those, they're automatically converted to the new style behind the scenes. Just happens to only support ED & RSA keys because of how we used to discerned those two.

bgins · 2022-08-11T14:36:01Z

One thing that I tried that failed was updating a user public key from Ed25519 to secp256k1. Not sure if that is a feature that we want.

Hmm, that should be possible. How did it fail?

It fails with a 400 with this message in the response

Error in $: Unable to decode Ed25519 PK because: CryptoFailed CryptoError_PublicKeySizeInvalid / BDTfGgRGmbA1Vjob/f8GU2ZjvY3KYP6f2DMor5Z7Te33wWPD48ik4nQeD7Qez549ue78xK5EBohBFkTogZeJ7bo=

The original key that was to be replaced was an Ed25519 key.

icidasset · 2022-08-16T15:54:59Z

Since updating public keys of the same type works I'm going to merge this in. I have to test transitioning between key types more extensively, but we can do that later (tracking in #629).

icidasset added 9 commits July 11, 2022 18:39

WIP

8901cc7

Try out other libsecp lib

f6e3319

Fix secp256k1 lib binding

43b454c

Add instances

5626b08

Fix instances & generate priv secp key with openssl

5a93cb7

Save public keys as a map in the db

ac4257f

SQL migration to new key column structure

bab32fa

Restore new structure on creation

f2e8d8e

Store the public key as JSON instead of a "map"

c36545b

icidasset commented Jul 25, 2022

View reviewed changes

hs-ucan/library/Crypto/Key/Asymmetric/Public/Types.hs Show resolved Hide resolved

icidasset commented Jul 25, 2022

View reviewed changes

hs-ucan/library/Crypto/Key/Asymmetric/Public/Types.hs Outdated Show resolved Hide resolved

icidasset added 7 commits July 27, 2022 17:19

Better function name and values for key type

4eb8563

Update fixtures

bcad4d2

Implement DID pieces

d22e33b

secp did test vectors

f887327

Update SQL migration to new types

929f3f6

WIP - UCAN integration

4ec1868

Adjust validation

77fcb88

icidasset commented Jul 29, 2022

View reviewed changes

hs-ucan/library/Web/UCAN/Signature/Types.hs Show resolved Hide resolved

icidasset commented Jul 29, 2022

View reviewed changes

hs-ucan/library/Web/UCAN/Types.hs Outdated Show resolved Hide resolved

icidasset commented Jul 29, 2022

View reviewed changes

hs-ucan/library/Web/UCAN/Types.hs Show resolved Hide resolved

Add Ethereum prefix and hash with Keccak 256

221d1f3

icidasset added 2 commits August 2, 2022 15:34

Parse old style of public key strings with JSON

d205891

Add libsecp256k1 to pkg-config-dependencies

0095a36

icidasset commented Aug 2, 2022

View reviewed changes

hs-ucan/package.yaml Outdated Show resolved Hide resolved

Add ubuntu deps

441b0cb

walkah reviewed Aug 2, 2022

View reviewed changes

flake.nix Outdated Show resolved Hide resolved

flake.nix Outdated Show resolved Hide resolved

icidasset added 3 commits August 4, 2022 17:30

Remove lib prefix

9863518

Create V3 routes to use the new type of public keys

036eb9b

Try running ldconfig after installing

65016bb

icidasset marked this pull request as ready for review August 5, 2022 17:25

icidasset requested review from matheus23, walkah and bgins August 5, 2022 17:31

walkah requested a review from avivash August 5, 2022 17:32

avivash reviewed Aug 5, 2022

View reviewed changes

hs-ucan/library/Web/UCAN/Internal/Orphanage/Secp256k1/PublicKey.hs Outdated Show resolved Hide resolved

icidasset and others added 5 commits August 8, 2022 11:40

Fix comment typo in hs-ucan/library/Web/UCAN/Internal/Orphanage/Secp2…

b78edab

…56k1/PublicKey.hs Co-authored-by: Andrew Vivash <[email protected]>

Save uncompressed public keys in the database

513520b

Check v value when checking secp256k1 signatures

fc1a365

Typo

cc68ce8

Use SHA256 hash for non-ETH keys

bccb129

icidasset mentioned this pull request Aug 8, 2022

Add support for an Ethereum PK elliptic curve #626

Closed

bgins reviewed Aug 10, 2022

View reviewed changes

bgins approved these changes Aug 10, 2022

View reviewed changes

icidasset merged commit 8b47050 into main Aug 16, 2022

icidasset deleted the icidasset/secp-did branch August 16, 2022 15:55

walkah mentioned this pull request Aug 31, 2022

Bump server version #630

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add secp256k1 key support for did:key #625

Add secp256k1 key support for did:key #625

icidasset commented Jul 25, 2022 •

edited

Loading

icidasset Jul 29, 2022 •

edited

Loading

icidasset commented Jul 29, 2022

icidasset commented Jul 29, 2022

walkah left a comment

avivash Aug 5, 2022

icidasset Aug 5, 2022

avivash Aug 5, 2022

icidasset Aug 8, 2022

avivash Aug 9, 2022

icidasset Aug 9, 2022

icidasset commented Aug 9, 2022

avivash commented Aug 10, 2022

bgins commented Aug 10, 2022

avivash commented Aug 10, 2022

bgins Aug 10, 2022

icidasset Aug 11, 2022

bgins left a comment •

edited

Loading

bgins commented Aug 10, 2022

walkah commented Aug 10, 2022

icidasset commented Aug 11, 2022

bgins commented Aug 11, 2022

icidasset commented Aug 16, 2022

	-- but since Ethereum (or Metamask?) often has a `v` value
	-- but since Ethereum often has a `v` value

Add secp256k1 key support for did:key #625

Add secp256k1 key support for did:key #625

Conversation

icidasset commented Jul 25, 2022 • edited Loading

Public key types

Migration

SECP256K1 UCANs

Ethereum signatures

icidasset Jul 29, 2022 • edited Loading

Choose a reason for hiding this comment

icidasset commented Jul 29, 2022

icidasset commented Jul 29, 2022

walkah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

icidasset commented Aug 9, 2022

avivash commented Aug 10, 2022

bgins commented Aug 10, 2022

avivash commented Aug 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgins left a comment • edited Loading

Choose a reason for hiding this comment

bgins commented Aug 10, 2022

walkah commented Aug 10, 2022

icidasset commented Aug 11, 2022

bgins commented Aug 11, 2022

icidasset commented Aug 16, 2022

icidasset commented Jul 25, 2022 •

edited

Loading

icidasset Jul 29, 2022 •

edited

Loading

bgins left a comment •

edited

Loading