-
Notifications
You must be signed in to change notification settings - Fork 280
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
Peer ID Calculation History And Resolution #138
Comments
Shouldn't these two be reversed? .18 would use sha256 and .19-dev would use inline. |
No. At the moment, 0.4.18 will use inlining, 0.4.16 and 0.4.19-dev won't. That's the issue. |
These are incompatible (without embedding hash function in the protobuf):
Edit: moved to #139 |
Hence #111. |
While I agree with the goal (I struggled with encryption key management in one of my experiments), I'm not sure it can be done securely. For Ed25519 to Curve25519 the community currently thinks it's likely to be secure but I haven't found a formal proof of that yet, which is why the golang library doesn't even offer to convert an Ed25519 key to a Curve25519 point. And even if that turns out to be secure for the specific case of Ed25519 keys, it's likely to be insecure for other types of keys so we shouldn't encourage users to use the peer ID key for encryption. I think some kind of handshake is preferable, as it also provides nice features like perfect forward secrecy (at the expense of performance unfortunately). |
@t-bast What is an Ed25519 signing key? |
Ed25519 is only a signing scheme (see here), not an encryption scheme. |
I see, so to rephrase what you said before: the public key (Ed25519) in the PeerID should only be used as a signing key |
That's exactly it. We'll need another mechanism to fetch the user's encryption key (in my small experiment users publish their encryption keys on the DHT and sign it with their peer key). |
I agree. We'll have to enable this on a per-key (type) basis. We don't currently support encryption with these keys for precisely this reason. |
If you ever do find a satisfactory solution, I'd love to hear about it. |
@t-bast @Stebalien This is not possible without in-lining. |
Thanks for pointing out that document! We've been discussing doing something like it in ipfs/notes#291 (comment). Note, that doesn't actually require (as far as I can tell) embedding the public key in the peer ID. Instead of deriving the encryption key from the public key, we'd derive it from the peer ID. |
The reason I believe in-lining is required is because With in-line keys this becomes |
Regarding encryption, I think there's no free lunch. If you want to have long-term encryption keys, you lose forward secrecy. That's the reason most protocols don't have long term encryption keys or rotate them regularly (tor's hidden services' master key used to derive the address is just a signing key, not an encryption key). I think forward secrecy is important, which means there will always be a handshake/roundtrips to retrieve an encryption key for a given peer. We'll just have to live with it and make it efficient enough. TLS 1.3's 0-RTT is a good way to mitigate the performance issue (for actions that are not vulnerable to replay attacks). Having a look at Signal's double ratchet could be an inspiration. I also remembered that Signal seems to have a solution that can provide signature and encryption with a single key, which they call XEdDSA, but I need to re-read the protocol because I remember that there are some caveats. Otherwise the hidden service v3 spec is a very solid protocol (but not necessarily very performant) so it can also be a good starting point. |
@Stebalien so here's our plan for ob... As part of the rebase to 0.18 I've put in patches to our However, I would like to switch to inline keys in the future as not having to look up the public key simplifies our code quit a bit in various places. If you guys did absolutely nothing I would probably be OK with that as once enough of our peers upgrade to our latest release I can migrate everyone to inline keys and stop patching go-ipfs in However, I do think it makes sense to from an architecture standpoint to support both formats but it would seem like there might be easier solutions. For example you could patch
Also the way I patched 0.18 connections was to change the But overall from our perspective we'd like to make sure go-ipfs remains compatible with inline keys in the future as it would greatly improve our stuff to make the switch. |
Note: IPNS records now include the key if it isn't inlined into the ID.
This is still the goal and we'll make sure to support it no matter what.
Take a look at the "Affected Subsystems" section above. The real issue happens when accepting an inbound connection. In that case, the "acceptor" doesn't know how the dialer calculated their peer ID so they may end up calculating it differently. This can affect things like DHT lookups. |
@Stebalien can go into more detail on this or maybe link to the part of the affected code. I'm asking because in my code I've only patched |
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien could you link out to the affected "accepter" code? OB are looking to converge their code base with upstream and this is a blocker |
@parkan this isn't about any specific code, just how DHTs work. For example:
|
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
this is likely needed for `go-ipfs` compatibility due to how the two versions of libp2p handle peerids differently: - libp2p/specs#138 - https://github.com/libp2p/go-libp2p-core/blob/dc718fa4dab1866476fd9f379718fdd619455a4f/peer/peer.go#L23-L34 - https://github.com/libp2p/rust-libp2p/blob/eb7b7bd919b93e6acf00847c19d1a76c09016120/core/src/peer_id.rs#L62-L74
this is likely needed for `go-ipfs` compatibility due to how the two versions of libp2p handle peerids differently: - libp2p/specs#138 - https://github.com/libp2p/go-libp2p-core/blob/dc718fa4dab1866476fd9f379718fdd619455a4f/peer/peer.go#L23-L34 - https://github.com/libp2p/rust-libp2p/blob/eb7b7bd919b93e6acf00847c19d1a76c09016120/core/src/peer_id.rs#L62-L74
Resolution: Keep the current peer ID calculation logic, complete with automatic key inlining. See the reasoning in #111 (comment). I'm going to leave this issue open as a place to discuss migrating to the new system. |
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
I am closing this issue as (a) there is a resolution and (b) there have been no discussions on migration in the past year. Please comment in case you would like to revive this thread. Thanks @Stebalien for the very detailed initial description and thanks to everyone involved along the way. |
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com> This commit was moved from ipfs/interface-go-ipfs-core@67fd754
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com> This commit was moved from ipfs/go-namesys@6913730
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com> This commit was moved from ipfs/kubo@2c93eef
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com> This commit was moved from ipfs/interface-go-ipfs-core@67fd754
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com> This commit was moved from ipfs/interface-go-ipfs-core@67fd754 This commit was moved from ipfs/boxo@132387d
Switch _back_ to the 0.4.18 style of peer IDs while we figure things out. See libp2p/specs#138. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com> This commit was moved from ipfs/interface-go-ipfs-core@67fd754 This commit was moved from ipfs/boxo@132387d
Forks off #111 to focus on the backwards compatibility issue instead of the CID/PID design space.
Summary
Peer ID calculation has changed a couple of times over the past year.
a. Textile is also using ed25519 keys and is relying on 0.4.18 behavior.
b. OpenBazaar is using a forked go-ipfs network. We are adding a package-level flag so they can restore pre-0.4.16 behavior in their forked build.
PR: libp2p/go-libp2p-peer#42
Goals
This all grew out of several requirements and wants:
Requirements
Wants
Definitions
public key can be extracted.
Note: "inlining" peer IDs look like
1...
, sha256 peer IDs look likeQm...
.Events
Below is an exhaustive history of this issue:
to embed them in peer IDs
(implement ed25519 support go-libp2p-crypto#5). However, we punted on
that. We never added support to go-ipfs
(Allow option to use ed25519 ipfs/kubo#3625).
libp2p/go-libp2p-peer#15) that embedded ed25519 keys. Unfortunately, that
wasn't usable because everyone needs to compute the same peer ID.
switched to automatically embedding keys shorter than 42 bytes into peer IDs.
This way, everyone would deterministically calculate the same peer ID. We did
this by using the "identity" hash function instead of sha256.
calculation had changed.
call, we decided to revert 5. We were under the
impression that nobody else was using ed25519 keys given that go-ipfs doesn't
provide a way to generate ed25519 keys.
trying to connect two nodes. @whyrusleeping tried reproducing it got the
error
dial attempt failed: <peer.ID Qm*yNGz7a> --> <peer.ID 12*FrJvar>
dialattempt failed: connected to wrong peer". This is the inverse of the issue
OpenBazaar was having.
having, they're seeing timeouts:
dial attempt failed: <peer.ID 12*xWYT4W> --> <peer.ID 12*YsNMRE> dial attempt failed: context deadline exceeded
anddial attempt failed: <peer.ID 12*xWYT4W> --> <peer.ID 12*YsNMRE> dial attempt failed: dial tcp4 13.57.23.210:4001: i/o timeout
. The latter lookslike it comes from add a TCP connect timeout go-tcp-transport#24.
This issue covers the peer ID issue, not the timeout issues.
Current State
The latest go-ipfs and go-libp2p masters are not inlining keys into peer IDs.They are now (again) inlining keys.don't actually need to interoperate with the rest of the network.
Affected Subsystems
This covers the affected subsystems and some hacky fixes that I don't recommend.
They're only there to illustrate the issue.
Outbound Connection
When establishing an outbound connection, go-libp2p will:
Textile is seeing this fail after updating go-libp2p because they're passing a
peer ID created using the identity hash function into go-libp2p while
go-libp2p-peer is calculating it using the sha256 multihash.
Example Fix: This could be fixed by a hack to convert a peer ID created using
the identity hash function to a sha256 one.
Inbound Connection
When receiving an inbound connection, we compute the peer's ID from their public key. That means:
Example Fix: In practice, this "just works". That is, we use our version of the
peer's ID internally and don't really care what the other side thinks it's ID
is.
DHT
The DHT is the first place where we really care about calculating the same peer
IDs. If we don't,
FindPeer
breaks.Let's say there's some peer A that wants to connect to a peer B. Let's assume
that peer A and B have this peer ID inlining enabled but the rest of the network
doesn't.
When peer A tries to connect to peer B, it'll walk the DHT looking asking nodes if they either:
actually connected to peer B will respond.
In this case,
to peer B. That is, (1) will work.
Example Fix: So, if we have an inlining peer ID, we can extract the key and
compute the sha256 peer ID and try to look that up in the DHT. Unfortunately,
we can't go the other way. This fix would also be really hacky.
IPNS
I don't believe IPNS is affected but I haven't thought through it thoroughly. At
worst, we'd have to apply a fix similar to the outbound connection fix.
PubSub
Same as IPNS.
Multibase
Unfortunately, this whole issue also relates to the ask from IPFS In Web
Browsers to make IPNS (and peer IDs) use multibase. If we just have
Qm...
IDs, we can avoid allocating
Q
as a multibase prefix and we'll be fine.However, inlining peer IDs start with
12
and1
is already a valid (albeituseless) multibase prefix for unary.
The real worry is that if we allow arbitrary mulithash functions, we need to
tackle the multibase issue before we start running into collisions. That is,
hash codes such that
Base58Encode([hash code])
maps to some useful multibaseprefix.
The text was updated successfully, but these errors were encountered: