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

IPNS: support Ethereum wallet signing #323

Open
paulgmiller opened this issue Sep 21, 2022 · 10 comments
Open

IPNS: support Ethereum wallet signing #323

paulgmiller opened this issue Sep 21, 2022 · 10 comments
Labels
kind/enhancement A net-new feature or an improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up

Comments

@paulgmiller
Copy link

paulgmiller commented Sep 21, 2022

I think it would be pretty useful if ipns recognized the signing prefix used by ethereum wallets. That way users with metamask or other wallets could sign ipns records to send to trusted nodes or providers like cloudflare, web3-storage, pinata without having to manage their own set of keys.

I think this would open up some interesting possibilities of distributed blogs other frequently updated content.

I think this would be pretty possilble to do in both go|js-ipns. Been poking around with a branch that can validate thos sigs
But if desired not sure if it would be best to

  1. Always check the eth prefix.
  2. Make this a new nodeid in libp2p so we could go off its type.
  3. use some other data in the ipns record to indicate it's a eth signed record.

Happy to try and contribute if wanted and someone is willing to guide preferences.

Some background here

@paulgmiller paulgmiller added the need/triage Needs initial labeling and prioritization label Sep 21, 2022
@lidel
Copy link
Member

lidel commented Sep 21, 2022

Thank you for filling this.
go-ipns repository is not the right place to propose this change.
I am moving this to ipfs/specs repo :)


FYSA the modern IPNS specification, including details how records are created and validated, is being documented in #319. Give it a read – that PR documents how go-ipns and js-ipns work today.

See "Backward Compatibility" section to understand the constraints and our policy around not breaking legacy clients (tldr: IoT device must be able to fetch and validate IPNS record with firmware update).

Something you may find useful, are Extensible IPNS Records.
IPNS records have IpnsEntry.data field, which is an extensible DAG-CBOR document.
Implementations are free to put additional data there, such as additional audit information.

Our libraries do not expose easy way of adding fields to IpnsEntry.data yet, but the specification and the wire format already support it. Sounds like this is the best place for storing these additional signatures.

@ipfs ipfs deleted a comment from welcome bot Sep 21, 2022
@lidel lidel transferred this issue from ipfs/go-ipns Sep 21, 2022
@paulgmiller
Copy link
Author

THanks for the link to #319 the existing spec didn't say much about data (nor did it talk about the v2 sig thats in the code). So excited to give this a look!

@paulgmiller
Copy link
Author

Left a comment on #319 could totally see how we could add extra CBOR fields.

SignaturePrework : Ethereum
or seperate fields
SignatureHash: None | keccak256
SignaturePrefix: \x19Ethereum Signed Message:\n

But those would be replacements not additions to signature v2 for those records becasue we wallets won't give out access to the private keys in any reasonable UI flow.

Would it be fine for only some nodes accept and circulate these records? Is it different from adding a new Public Key type in the future?

Thanks again.

ethereum/go-ethereum@b59c839

@guseggert guseggert added P2 Medium: Good to have, but can wait until someone steps up kind/enhancement A net-new feature or an improvement to an existing feature and removed need/triage Needs initial labeling and prioritization labels Oct 6, 2022
@lidel
Copy link
Member

lidel commented Oct 23, 2022

Would it be fine for only some nodes accept and circulate these records?

I mean, the network does not care – it will simply ignore these additional fields.
If the IPNS signature is invalid, records are simply ignored. Same for unknown key types.

It is more a question of your use case: If you are creating software that will both create and validate this additional metadata, then you can do whatever you want: as long you also include a valid IPNS signature that is in the spec, DHT nodes and PubSub routers will accept IPNS PUT your software does.

@lidel
Copy link
Member

lidel commented Dec 13, 2022

@paulgmiller, some additional notes around "signing IPNS record with Metamask" use case.
Apologies for wall of text, but I wanted to give a comprehensive explainer that we could reference in the future.

On signing IPNS records with Metamask

Discussed this with @2color today in the context of ipfs/js-ipns#192 (making it possible to sign valid IPNS records with secp256k1 keys outside of IPNS node) and the underlying problem is that even though IPNS supports signing records with secp256k1 keys, the way these signatures are performed is different than one (out of four+ ways) signing happens in Ethereum.

Multiple types of secp256k1 signatures

iiuc Ethereum and IPNS use different hash functions and prefix signed payload with different bytes:

Ethereum (metamask) signature with secp256k1 (one of four options?) IPNS signature (V2) with secp256k1
signA(keccak256(\x19Ethereum Signed Message:\n + len(data) + data)) sign_bip62_der(sha256(ipns-signature: + data))
signA() defined by EIP-712 (v4)
(but it is similar story for 4 other types listed here)
sign_bip62_der() defined by BIP0062, see libp2p specs

IPNS should not depend on any blockchain specifics

Given the above, we have incompatible signatures.
One could argue IPNS spec could be extended to with Ethereum-specific signatures, but why should we tie IPNS spec to Ethereum-specific signatures, and not Some Other Blockchain?

This raises even more questions:

  • how to signal when different signature for the same key type should be used?
  • Should we add multiple versions of the same key type, to remove this ambiguity around how to create signature?
  • How do we make the decision which chain-specific-signature-flavour is "worthy" and which one is not?
  • Why do we shift the implementation burden to IPNS implementers who don't care about all these blockchains?

Finally, Ethereum ecosystem seems to have competing signature standards: there are 4 standards, and they already discuss adding v5. This does not bring a lot of confidence into stability and interoperability.

👉 I think all these questions and rabbit holes should be avoided. IPNS signatures must be chain-agnostic.

IPNS is routing-agnostic (records can be resolved via DHT, PubSub, and soon Gateways).

We allow multiple key types, but the way signatures are created must remain IPFS-specific to

  1. avoid the discussion which blockchain is "worthy",
  2. avoid ballooning complexity on the client side by requiring support for multiple key types (and / or signature types)
  3. ensure signatures can't be used outside of IPNS record context

What are the next steps?

For "signing IPNS with Metamask" use case, I see three paths forward:

  • Dual signing with two sets of keys
    (no change in Metamask, no change in IPNS spec, but second signature is only used by Ethereum-aware clients).

    • Metamask signs DAG-CBOR in IpnsEntry.data, and we then put that signature inside something like IpnsEntry.data[ethereum_personal_sign] itself and sign entire IpnsEntry.data with IPNS-specific key and prefix and store result in signatureV2
    • IPFS clients will ignore Ethereum-specific signature present in IpnsEntry.data, but clients who care about Ethereum identity could verify this second signature for additional confidence or UX.
    • 🟢 This could work today, requires minimal work in libraries like js-ipns to expose ability to modify CBOR in IpnsEntry.data before signing. And it would work with Ledger (which seems to only support personal_sign method)
  • Signing using current IPNS signatureV2 spec
    (no change in IPNS spec, requires Metamask update)

    • For signed records to be interpreted as valid by IPFS nodes today, one needs to create a record with IPNS-spec compliant signatureV2), which for secp256k1 means doing: sign_bip62_der(sha256(ipns-signature: + data))
    • 🟢 This could be implemented as another (they already support, so 🤷) in Metamask, as long their software wallet is used.
    • 🔴 This won't be possible with Ledger and other hardware wallets that only implement personal_sign method – they would have to implement IPNS-specific signing method.
  • Modify IPNS specification
    (hard way, requires changes across specs and IPNS implementations, and if it is chain-agnostic, also Metamask)

    • Leverage the IPIP process: new chain-agnostic signature standard(s) could be proposed as IPIP
    • This is long term work, requires buy-in from multiple stakeholders to ensure both adoption and interoperability.

@lidel lidel changed the title [Feature] Support ethereum wallet signing. Support Ethereum wallet signing Dec 13, 2022
@lidel lidel changed the title Support Ethereum wallet signing IPNS: support Ethereum wallet signing Dec 14, 2022
@paulgmiller
Copy link
Author

This is a great write up and way more than I expected from this issue. Thank you very much. I totally get the concerns about ipns not chaining itself to any of the many blockchain standards maybe if that stabilizes it coming years there might be a defacto standard but probably too early to tell.

Speaking towards the next steps.

I think the main issue with "dual signing" is that ipfs nodes will only keep the latest revision so an ipfs node has to generate a new key for each "eth/metamask" user that wants to use it otherwise it will only be able to serve one forieng user. So if that user moves between nodes it'll be lost what public key to look up. So you need another global source of truth like ens/dns to be updated when that happens.

For "using current ipns spec" you need not just metamask but all wallets to pick this up doable but very long term in my mind.

My current plan is to just create an application specific version of ipns that publishes via pubsub so apps that want this can get it. The BIG downside of this is only apps that know about it participate. You can't get all the ipfs nodes to participate. But it does let us get ipns pointers passed around and we can come back to "modify ipns spec" if there is every any successful app here.

Thanks again!

@bumblefudge
Copy link
Collaborator

Wow, there's a lot to take in here! I would recommend, before picking a path and getting to work, doing a tiny bit of research on the metamask Snaps project and its custom signing and deterministic key derivation capabilties! Snaps are basically MM plug-ins, and AFAIK the preferred way for external teams to extend metamask signing and key handling capabilities...

@2color
Copy link
Member

2color commented Dec 20, 2022

To elaborate on (or summarise) what @lidel mentioned, all current signing methods in the Ethereum ecosystem ((https://eips.ethereum.org/EIPS/eip-191 and https://eips.ethereum.org/EIPS/eip-712)) are unsuitable for interoperability with IPNS for two reasons:

  • They hash using keccak256 before signing — this hash function is currently not supported by the IPFS/libp2p libraries
  • For security reasons, signed messages have an Ethereum specific prefix: sign(keccak256("\x19Ethereum Signed Message:\n" + len(message) + message))). for security reasons.

This leads to the three options laid out:

I'll be getting in touch with the Metamask folks via CASA to make them aware and investigate some possible future paths.

@ritave
Copy link

ritave commented Jan 12, 2023

Hello from an engineer working on MetaMask Snaps.

There have been quite a few requests to us for different cryptographic primitives and signature schemes, including JWT tokens, Bitcoin signatures, IPLD through CACAO, plain RSA, etc.

We can't support all of them due to their sheer count.

What we're trying to achieve is to allow the community to agree on common standards themselves and then allow people to extend MetaMask using Snaps to support that specific use-case.

We're allowing Snaps access to raw private key entropy so a Snap that provides IPNS signatures is possible.


In short, I'm suggesting it might be easier to write a plugin for MetaMask to support IPNS rather than updating the IPNS spec to support MetaMask, and our team will do everything what we can support you on that route.

@2color
Copy link
Member

2color commented Jan 16, 2023

Thanks for the update @ritave

I suppose the Filsnap might be worth looking at for how to do so signing with Metamask Snaps, e.g. https://github.com/ChainSafe/filsnap/blob/master/packages/snap/src/rpc/signMessage.ts#L102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or an improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

7 participants
@lidel @guseggert @paulgmiller @ritave @2color @bumblefudge and others