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

Use secp256k1 keys in IPFS/libp2p instead of ed25519 #342

Closed
Wondertan opened this issue May 19, 2021 · 18 comments
Closed

Use secp256k1 keys in IPFS/libp2p instead of ed25519 #342

Wondertan opened this issue May 19, 2021 · 18 comments

Comments

@Wondertan
Copy link
Member

Wondertan commented May 19, 2021

ref: #340 (comment)

@Wondertan Wondertan self-assigned this May 19, 2021
@liamsi
Copy link
Member

liamsi commented May 19, 2021

Also investigate if we should use the same key as the consensus key then (cc @adlerjohn).

@adlerjohn
Copy link
Member

I don't think it's strictly necessary to use the same keys as consensus keys, but using the same keys for everything will probably simplify things, so sure.

@liamsi
Copy link
Member

liamsi commented May 19, 2021

Copying over part of my comment from: #340 (comment)

I'm back and fort on this. I do not care too much about secp vs ed25519 tbh but the general question if we should have one key for all purpose or several keys for different purposes. It might be not a bad idea to make it non-trivial to discover which validator runs which ipfs node for instance.

@liamsi
Copy link
Member

liamsi commented May 22, 2021

Any thoughts on the above @adlerjohn ? If the key is reused it is trivial to identify which validator runs which IPFS node.

@adlerjohn
Copy link
Member

Oh by "key" do you mean the actual secret key, or just the signing scheme. If the actual secret key, you probably don't want IPFS touching that.

@liamsi
Copy link
Member

liamsi commented May 22, 2021

@adlerjohn
Copy link
Member

Does that PKI require the IPFS process to know about your secret key? Or just a public key? I imagine it doesn't work without a secret key, otherwise anyone could spoof. In that case, I'd be very hesitant to use the same key for both being a validator and IPFS.

@liamsi
Copy link
Member

liamsi commented May 22, 2021

Yeah, it does not make any sense if IPFS does only know about the pubkey ...

@liamsi
Copy link
Member

liamsi commented May 22, 2021

@Wondertan I've unassigned you because I think this one has super low priority.

@Wondertan
Copy link
Member Author

@liamsi, ok
Although, I guess we can return to this question once we fully absorb IPFS, not to share Sk to IPFS process.

@liamsi
Copy link
Member

liamsi commented May 22, 2021

My original concern was not about sharing the SK to the ipfs process (@adlerjohn brought that up), but more the fact that identifying a validator will be the same as identifying the Ipfs node it is running (we should specify if that is a property we want). I think that is the question we should revisit first.

I'm against re-using the same key. the consensus key should only be used to sign consensus messages. e.g. the key could be on a remote signer that only understands and signs consensus messages. Shifting away from this might have the impact that we can't re-use existing infrastructure for instance.

@liamsi
Copy link
Member

liamsi commented Jun 4, 2021

This is not entirely the right place but it fits best here and I don't want to open a new issue for this: the tendermint p2p node key could and probably should be re-used in case we end up using libp2p more:
https://github.com/lazyledger/lazyledger-core/blob/94c485374faa5948df3f6064ecbe08f4b83c4b00/p2p/key.go#L24-L31

This also defaults to ed25519 btw: https://github.com/lazyledger/lazyledger-core/blob/94c485374faa5948df3f6064ecbe08f4b83c4b00/p2p/key.go#L79

related to: #389

@Wondertan
Copy link
Member Author

In order to match malicious nodes in Push DA with consensus/validaotor nodes, we need to have this. PubSub encapsulates messages singing and verification and not reinvent the wheel including this also makes sense

@liamsi
Copy link
Member

liamsi commented Jun 15, 2021

In order to match malicious nodes in Push DA with consensus/validaotor nodes, we need to have this.

Matching those only makes sense if "malicious nodes in Push DA" trigger a slashable event. Otherwise we don't need to match both. If it is not slashable it does not need to be attributable to a validator but is only a p2p-layer/networking concern.

@Wondertan
Copy link
Member Author

@liamsi, I reread my statement and understood that it misses an essential part of the explanation, so expanding on this:

I was wonderting about DoS vector concerns and proposing this to be part of the solution. But how? Luckily, we have a deterministic round-robin consensus where we know who is the proposer in advance. This allows the consensus layer to notify the p2p layer about shares origin, so it will just drop messages from any other origin preventing DoSing. Furthermore, if the expected origin act maliciously, e.g., provided a sample that is not part of the DataHash Merkle tree, we can then act accordingly on the consensus layer: recheck received DataHash is originated from the malicious validator and vote for nil or execute some slashing mechanism with the committee.

Going technical, I think of introducing options for Get/Retrieve data in the DA interface. For Push schema it will check PubSub messages origin, and for Pull it will ensure that we are connected to the origin peer.

Hope that makes sense.

/cc @musalbas for credit, as you initially pointed me to that on the previous dev call.

@liamsi
Copy link
Member

liamsi commented Jun 25, 2021

Luckily, we have a deterministic round-robin consensus where we know who is the proposer in advance. This allows the consensus layer to notify the p2p layer about shares origin, so it will just drop messages from any other origin preventing DoSing.

This is not how simple gossip networks work though: only because a proposer is proposing a block it does not mean you will receive that data directly from that proposer (but rather from some peer you are connected to, could be the proposer but could be any validator / tendermint full node).

Let's properly define all kinds of fraud proofs we want in the specification, together with slashable events they trigger (if any). Then we can decide on how to proceed with the slashing part. If we want to slash gossiping byzantine shares, then yes, the validator key should also be used for the p2p layer to make this attributable. Intuitively, I still think this is not a good idea and the consensus signing key should only be used for consensus messages and not for p2p concerns.

@Wondertan
Copy link
Member Author

This is not how simple gossip networks work though: only because a proposer is proposing a block it does not mean you will receive that data directly from that proposer (but rather from some peer you are connected to, could be the proposer but could be any validator / tendermint full node).

Idk specifically about Tm’s gossiping, but it is normal to sign a message before gossiping, so any relayer or recipient knows its origin. The recipient should both know the origin and related it got the message from. There is nothing complex here.

Intuitively, I still think this is not a good idea and the consensus signing key should only be used for consensus messages and not for p2p concerns.

I fully agreed with such intuition, but this specific case changed my opinion slightly. It's clear that if running deterministic consensus with gossiping while tracking message origin allows hardening of security. Signing gossip messages with consensus key though doesn't mean using them everywhere, e.g., transport handshakes, etc

@liamsi
Copy link
Member

liamsi commented Aug 16, 2021

Closing this as this is no longer relevant. The choice of keys can be argued in an issue/ADR in https://github.com/celestiaorg/celestia-node instead.

@liamsi liamsi closed this as completed Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants