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

[Network] Verify originID against libp2p ID #1115

Closed
synzhu opened this issue Aug 10, 2021 · 2 comments · Fixed by #1129
Closed

[Network] Verify originID against libp2p ID #1115

synzhu opened this issue Aug 10, 2021 · 2 comments · Fixed by #1129
Assignees

Comments

@synzhu
Copy link
Contributor

synzhu commented Aug 10, 2021

Today, the network layer does not verify the OriginID of a message, and it passes it directly to the engines.

There are many reasons that this is bad (opens the door to impersonation attacks), and now with unstaked nodes it is even more important to address this.

  • The verification for staked nodes can be done by checking the OriginID against the From field of the raw message we get from libp2p. Given a Flow ID, we should be able to deduce the libp2p peer ID from the protocol state.
  • For unstaked nodes, we will do the validation by comparing the given OriginID against the result of whatever deterministic libp2p ID -> Flow ID mapping we end up deciding on.
  • For staked AN's, we will validate messages from the unstaked network inside the libp2p topic validator, but using the same method as above.

As a result, we may need to do some refactoring so that the network layer of the code is aware of whether it is a staked or unstaked network, so it can choose which verification logic to use from above. Alternatively, we can implement some message validators that do this validation.

We can write a test to ensure that this is all working:

  • Start up two nodes, and from one of them send a message with an incorrect origin ID
  • Check that this message is discarded by the receiving node.
@synzhu
Copy link
Contributor Author

synzhu commented Aug 11, 2021

@huitseeker You can probably just implement the first bullet point out of the above. The second and third bullet points may not even be strictly necessary, since on the unstaked network the origin ID will be derived from the libp2p ID, so we could potentially just ignore whatever Origin ID value is inside the actual message.

@huitseeker
Copy link
Contributor

This was actually not fixed in 1129, as we decided to defer this fix to a further PR.

@huitseeker huitseeker reopened this Aug 18, 2021
bors bot added a commit that referenced this issue Aug 19, 2021
1138: skip connecting to invalid identities in the identity table and log error instead of fatal r=vishalchangrani a=vishalchangrani

Found one more place where it was `log.fatal` on a bad identity.


1163: [network] Check the OriginID of a libp2p message corresponds to its authenticated source r=huitseeker a=huitseeker

Contributes to #1115.
Follow-up of #1129.

Edit: there was no problem with the integration tests, 🤦 

However, TestMiddlewareTestSuit/TestUnsubscribe is still flaky based on the delay in the last step (sending the message once a mesh has formed). But it is problematic on master, see [[1]](https://github.com/onflow/flow-go/runs/3357056741) [[2]](https://github.com/onflow/flow-go/runs/3356508175), so this is not relevant to this PR per se.

Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
bors bot added a commit that referenced this issue Aug 19, 2021
1028: adding secure-rpc-addr to access node systemd file r=vishalchangrani a=vishalchangrani



1163: [network] Check the OriginID of a libp2p message corresponds to its authenticated source r=huitseeker a=huitseeker

Contributes to #1115.
Follow-up of #1129.

Edit: there was no problem with the integration tests, 🤦 

However, TestMiddlewareTestSuit/TestUnsubscribe is still flaky based on the delay in the last step (sending the message once a mesh has formed). But it is problematic on master, see [[1]](https://github.com/onflow/flow-go/runs/3357056741) [[2]](https://github.com/onflow/flow-go/runs/3356508175), so this is not relevant to this PR per se.

Co-authored-by: vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
bors bot added a commit that referenced this issue Aug 19, 2021
1028: adding secure-rpc-addr to access node systemd file r=vishalchangrani a=vishalchangrani



1163: [network] Check the OriginID of a libp2p message corresponds to its authenticated source r=huitseeker a=huitseeker

Contributes to #1115.
Follow-up of #1129.

Edit: there was no problem with the integration tests, 🤦 

However, TestMiddlewareTestSuit/TestUnsubscribe is still flaky based on the delay in the last step (sending the message once a mesh has formed). But it is problematic on master, see [[1]](https://github.com/onflow/flow-go/runs/3357056741) [[2]](https://github.com/onflow/flow-go/runs/3356508175), so this is not relevant to this PR per se.

Co-authored-by: vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
bors bot added a commit that referenced this issue Aug 19, 2021
1163: [network] Check the OriginID of a libp2p message corresponds to its authenticated source r=huitseeker a=huitseeker

Contributes to #1115.
Follow-up of #1129.

Edit: there was no problem with the integration tests, 🤦 

However, TestMiddlewareTestSuit/TestUnsubscribe is still flaky based on the delay in the last step (sending the message once a mesh has formed). But it is problematic on master, see [[1]](https://github.com/onflow/flow-go/runs/3357056741) [[2]](https://github.com/onflow/flow-go/runs/3356508175), so this is not relevant to this PR per se.

Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
bors bot added a commit that referenced this issue Aug 19, 2021
1163: [network] Check the OriginID of a libp2p message corresponds to its authenticated source r=huitseeker a=huitseeker

Contributes to #1115.
Follow-up of #1129.

Edit: there was no problem with the integration tests, 🤦 

However, TestMiddlewareTestSuit/TestUnsubscribe is still flaky based on the delay in the last step (sending the message once a mesh has formed). But it is problematic on master, see [[1]](https://github.com/onflow/flow-go/runs/3357056741) [[2]](https://github.com/onflow/flow-go/runs/3356508175), so this is not relevant to this PR per se.

Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
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

Successfully merging a pull request may close this issue.

4 participants