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

Switch to ed25519 keys by default so that we can reasonably store signed content records #6916

Closed
3 of 5 tasks
jacobheun opened this issue Feb 20, 2020 · 16 comments
Closed
3 of 5 tasks
Labels
epic exp/expert Having worked on the specific codebase is important kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP status/in-progress In progress topic/cidv1b32 Topic cidv1b32 topic/ed25519 Issues related to ed25519 Peer IDs topic/encryption Topic encryption topic/gateway Topic gateway
Milestone

Comments

@jacobheun
Copy link
Contributor

jacobheun commented Feb 20, 2020

To be confirmed:

  • Add support for specifying key type in ipfs init
  • Switch initialization to use ed25519 as the default instead of rsa

Success Criteria

  • Newly created IPFS nodes will use ed25519 keys by default
  • Users can rotate their existing keys to ed25519

What's Left

Mandatory:

Nice to Have:

@jacobheun jacobheun added the kind/enhancement A net-new feature or improvement to an existing feature label Feb 20, 2020
@jacobheun jacobheun added this to the go-ipfs 0.6 milestone Feb 20, 2020
@jacobheun jacobheun added the epic label Feb 20, 2020
@RubenKelevra
Copy link
Contributor

Will self also be changed to ed25519?

@jacobheun jacobheun added exp/expert Having worked on the specific codebase is important P0 Critical: Tackled by core team ASAP status/ready Ready to be worked topic/encryption Topic encryption labels Apr 20, 2020
@willscott
Copy link
Contributor

willscott commented Apr 20, 2020

Users can rotate their existing keys to ed25519

What does this mean, exactly?
It's easy enough to generate a new ed25519 key. is there / does there need to be a way to indicate that identity transition to peers? Enumerating what needs to change in a rotation would be useful.

@Stebalien
Copy link
Member

It means #5880. That is, users need to be able to "re-key" their node without changing anything else.

@Stebalien
Copy link
Member

@RubenKelevra yes. self is the node's ID. Note: we won't automatically change user keys, we're just (a) providing an easy way to do this and (b) changing the defaults for newly initialized nodes.

@RubenKelevra
Copy link
Contributor

@RubenKelevra yes. self is the node's ID. Note: we won't automatically change user keys, we're just (a) providing an easy way to do this and (b) changing the defaults for newly initialized nodes.

Will this be backwards compatible? I mean, can for example 0.4.23 handle nodes with ed25519 keys today?

@Stebalien
Copy link
Member

Yes.

@lidel
Copy link
Member

lidel commented May 11, 2020

When we switch to ed25519 by default, we should ensure that keys are always represented as CIDv1 in Base32Base36 with libp2p multicodec (see RFC/0001-text-peerid-cid.md).

FYSA subdomain support in our HTTP Gateway is already doing the conversion if someone uses anything other than CIDv1Base32:
https://github.com/ipfs/go-ipfs/blob/0114869d7e7d43038d1a05cd76fec22aa07f78ff/core/corehttp/hostname_test.go#L29-L30

https://github.com/ipfs/go-ipfs/blob/0114869d7e7d43038d1a05cd76fec22aa07f78ff/core/corehttp/hostname.go#L312-L343

@lidel lidel added topic/cidv1b32 Topic cidv1b32 topic/ed25519 Issues related to ed25519 Peer IDs labels May 11, 2020
@Stebalien
Copy link
Member

Stebalien commented May 12, 2020 via email

@lidel
Copy link
Member

lidel commented May 13, 2020

Main motivation is to make a single default change (rsa+b58 → ec+b32)
instead of two (rsa+b58 → ec+b58, and then ec+b58 → ec+b32).

Second motivation is for users to be able to copy ipns key and open it from gateway without the need for conversion step:


@Stebalien RFC/0001-text-peerid-cid.md is tracked in libp2p/specs#216 but I don't see multiaddr on the list – what remains to be done?

@Stebalien
Copy link
Member

We need to update https://github.com/multiformats/go-multiaddr/blob/8ae68cf376240f0db2decc6bed91d8c5bc198e75/transcoders.go#L293-L315 to allow CIDs. It's pretty trivial but, IIRC, the main concern was adding a dependency on CID (but that's no much of a concern).

However, I'm not sure we're ready to use this by default given that we only just started supporting it this last release while we've supported ed25519 keys for quite a while. On the other hand, we could just use it as a motivation to upgrade?

@lidel
Copy link
Member

lidel commented May 14, 2020

I'd prefer if we switch both defaults together to limit the number of revolutions in PeerID land, but world would not end if we punt it until we switch default CID representation.

That being said, I believe we should solve #7318 before switching to ed25519 :(

@lidel
Copy link
Member

lidel commented Jun 15, 2020

Quick updates on related changes:

  • JS: cids@0.8.2 library shipped with support for Base36, which solves the problem of representing ED25519 keys on subdomain gateways.
  • GO: go-ipfs 0.6.0 supports conversion to DNS-safe CIDv1 in Base36:
    $ ipfs cid format -v 1 --codec libp2p-key -b base36 z5AanNVJCxnNXk3KysfhKVHaJecKEY2RxgsrNaGb89fuzXVqjaEndN1
    k51qzi5uqu5dj16qyiq0tajolkojyl9qdkr254920wxv7ghtuwcz593tp69z9m
  • I've updated cid.ipfs.io and it now supports analysis of ED25519 PeerID as CIDv1 in Base36 such as k51qzi5uqu5dj16qyiq0tajolkojyl9qdkr254920wxv7ghtuwcz593tp69z9m

@lidel
Copy link
Member

lidel commented Jun 30, 2020

Subdomain gateway support for ED25519 being reviewed in #7441

@aschmahmann
Copy link
Contributor

@lidel as mentioned in https://github.com/ipfs/go-ipfs/pull/7441/files#r447624101 it sounds like we have a decision to make about whether all libp2p keys should be represented in base36 by default.

What do you think are the inputs into this decisions? It seems like doing it is useful because if we use base32 by default then people can't copy-paste them into web browser subdomains and won't be able to do a quick visual check on them. Is the downside related mostly to people being shocked that all of a sudden their existing RSA keys are being represented as a base36 libp2p-key instead of just QmXYZ? If so how are we planning to decide yes/no?

@lidel
Copy link
Member

lidel commented Jun 30, 2020

What do you think are the inputs into this decisions?

It is a mixture of hard constraints and nice-to-haves.
Below is my attempt to separate them.

Constraints:

  • (I believe) We need to use cidv1b36 for ed25519 everywhere to bridge the spec gap between CID and PeerID specs and make it easier for people to reason about those identifiers. The key here is using CIDv1 for ed25519 everywhere. Longer explainer below.

  • We are forced to use Base36 encoding for ed25519 because of subdomains (feat: support ED25519 at subdomain gw with TLS #7441).
    Due to this, it makes sense to use Base36 as the default for those keys, avoiding the conversion step.

Nice-to-haves:

  • When go-ipfs makes the switch to ed25519 by default, and those are encoded as CIDv1 in Base36, we may also switch the text representation of old RSA keys to use the same encoding. For the same of simplicity, improved UX and removal of pre-CIDv1 technical debt. But this is not a must.

Longer rationale below. (I'm sorry, it was late and I failed to make it shorter :()


Why we MUST use CIDv1B36 for ed25519

I think we need to recognize that switching to inlined ed25519 surfaces an edge case in our specs and introduces sever UX challenges if we keep text output as Multihash in Base58btc.

Namely, according to decoding algorithm in CID spec a CIDv0 is a Multihash in Base58btc starting with Qm. Everything else is CIDv1+.
That means an ed25519 Multihash starting with 1.. is NOT a CIDv0, and libraries will try to parse it as CIDv1+ resulting in errors.

To illustrate how bad it gets (defaults, go-ipfs 0.6):

  • ipfs key gen -t ed25519 produces libp2p-key as Multihash in Base58btc (12D3KooWP3ggTJV8LGckDHc4bVyXGhEWuBskoFyE6Rn2BJBqJtpa)
  • User tries to convert it to Base36 via ipfs cid command to use it in subdomain
  • ipfs cid does not recognise multihashes starting with anything other than Qm.. (CID spec says Qm is CIDv0, everything else is parsed as CIDv1+):
    $ ipfs cid format -v 1 --codec libp2p-key -b base36 12D3KooWP3ggTJV8LGckDHc4bVyXGhEWuBskoFyE6Rn2BJBqJtpa
    12D3KooWP3ggTJV8LGckDHc4bVyXGhEWuBskoFyE6Rn2BJBqJtpa: selected encoding not supported
    This is go-ipfs. I think it is safe to assume this problem will occur in other places/libs too.
  • Ok, so user tries to inspect the libp2p-key at cid.ipfs.io/#12D3Ko.. and it looks fine. JS app at cid.ipfs.io does not follow spec to the word, and identifies it as CIDv0. There is CIDv1Base32 version at the bottom. It is cool this works, but discrepancy between CLI and web tooling does not smell well.
    If they copy Base32, it can be converted to Base36 just fine:
    $ ipfs cid format -v 1 --codec libp2p-key -b base36 bafyaajaiaejcbrerngkbad43x2wkgyoewpfspx4dkebmw4iygvqupxmhlh5ljdy3
    k51qzi5uqu5dl2yn0d6xu8q5aqa61jh8zeyixz9tsju80n15ssiyew48912c63
    I mean.. not the best user experience ¯_(ツ)_/¯

I don't think we really have any alternative here, when we switch to ed25519 by default, we need to output them as CIDv1 in Base36.

Why we (probably) SHOULD use CIDv1B36 for all libp2p-keys

In short: devexp, simplicity, removes cognitive overhead.
People could use regular CID library for extracting libp2p-key multihash.

Hardcoding check for Qm was a thing for a long time, and now that we have had CIDs for a few years developers assume every identifier in ecosystem is a CID. Many use CID parser for Qm.. PeerIDs already, and it works only because 1.. ones are rare.

I'm mildly in favor of switching text representation of all keys to CIDv1 Base36 (including RSA):

  • Representing PeerID/libp2p-key as CIDv1 will remove the surface for bugs like the one described above, and free us from that technical debt in user interfaces.
  • Added value is that CIDv1 will include libp2p-key codec, making all keys self-describing, improving UX even further
  • Downsides:
    • user may have Multihash PeerID in Base58btc hardcoded somewhere or do naive string comparison in code
      (I'd say it's better to break those setups sooner than later)
    • changing text representation from Base32 to Base36 in subdomain will change the Origin of IPNS websites using old RSA keys
      (same, I'd argue its better to do this change now that IPNS is not yet in its prime and the userbase is smaller)

@aschmahmann
Copy link
Contributor

Closed by #7579

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic exp/expert Having worked on the specific codebase is important kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP status/in-progress In progress topic/cidv1b32 Topic cidv1b32 topic/ed25519 Issues related to ed25519 Peer IDs topic/encryption Topic encryption topic/gateway Topic gateway
Projects
No open projects
Archived in project
Development

No branches or pull requests

6 participants