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

Extend RPC to unban peers #9918

Closed
gmilescu opened this issue May 4, 2022 · 9 comments
Closed

Extend RPC to unban peers #9918

gmilescu opened this issue May 4, 2022 · 9 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. Groomed icebox Node Node team P-low Priority: low T-node Team: issues relevant to the node experience team Task

Comments

@gmilescu
Copy link

gmilescu commented May 4, 2022

If some peers got banned, it's really hard to unban them right now. Given it sometimes happens due to issues with network, it would be good to be able to restore them back. Ideally this tool should be inside `neard` to not require another binary.

  • `neard peers` lists all peers in the database with information if they were banned
  • `neard unban ,` unbans list of peers

ND-40 created by None

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

mfornet commented:

To `unban` peers in this way nodes will need to restart to apply the changes. Another option is adding this function to our RPC, however it needs to be protected, for example, to make an unban request it should be signed with the secret key from the node.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

ilblackdragon commented:

I was thinking indeed stop & start node, as this is not normal operation.

But through RPC might be useful. If we are going that way, we would want to have API_KEY (signing is complicated and ideally calling RPCs should not happen from the node which means there won't be any keys). We can use http base authentication in RPC for this, e.g. `curl https://x:@rpc-host/unban`

But I don't think we should do this for Phase 1, to let's keep this in Icebox for now.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

mfornet commented:

What is the best workflow to generate and validate an `API_KEY`? It needs to be generated directly with our binary from the node.

I see two ways to do this:

  1. Random keys:
  • Generate an API_KEY on the fly and store its hash on a database (protected .txt/.json should work ok)
  • To check an API_KEY is valid just check its hash is on the database
  • To revoke an API_KEY remove it from the database
  1. Signed keys:
  • Create a new key pair (api_key_generator)
  • To generate an API_KEY do:
    • Generate a random number of length K
    • Sign this number using the `api_key_generator`
    • The generated API_KEY consist on the concatenation of both
  • To check an API_KEY is valid check the signature matches
  • To revoke an API_KEY change the api_key_generator (it will revoke all other existing API_KEY too).

I think, the first variant is strictly better.

Ok with doing this after phase 1\. This entire mechanism will be useful not only for ban/unban peers, but with other private functionalities we want to expose through RPC.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

frol commented:

Alternatively, we can implement a separate RPC server using a separate TCP port, by default listening on 127.0.0.1, so there is no way to suddenly expose the RPC to the outside world. Exposing this RPC then can be done with a reverse-proxy basic auth configuration or even a custom managing service. This way we don't need to roll out yet another key management.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

stale[bot] commented:

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

1 similar comment
@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

stale[bot] commented:

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

frol commented:

@janewang I want to bring this up to NodeX team awareness. I feel that there might be a need for granular administration of a running node through a dedicated admin RPC. We already have adversarial features hidden under a compilation flag, which might also be moved under this admin RPC.

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu
Copy link
Author

gmilescu commented May 4, 2022

bowenwang1996 commented:

> If some peers got banned, it's really hard to unban them right now.

That is actually not the case. You can change `ban_window` in config.json to a small number (such as 1s) to unban peers

by 557058:c020323a-70e4-4c07-9ccc-3ad89b1c02ec

@gmilescu gmilescu added the Node Node team label Oct 18, 2023
@gmilescu
Copy link
Author

Closing as duplicate of #2850.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. Groomed icebox Node Node team P-low Priority: low T-node Team: issues relevant to the node experience team Task
Projects
None yet
Development

No branches or pull requests

1 participant