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.

Sign up for GitHub

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

Jump to bottom

(feat): Allow binding RPC to only specific addresses #2824

Closed
adlerjohn opened this issue Oct 9, 2023 · 1 comment · Fixed by #2955
Closed

(feat): Allow binding RPC to only specific addresses #2824

adlerjohn opened this issue Oct 9, 2023 · 1 comment · Fixed by #2955
Assignees
@ramin
Labels
enhancement New feature or request kind:feat Attached to feature PRs

Comments

@adlerjohn
Copy link
Member

Implementation ideas

Currently the node will accept RPC requests from any IP (modulo authentication). Add the ability for the node to restrict which IPs it will listen to for requests, allowing localhost only by default.

From https://geth.ethereum.org/docs/fundamentals/command-line-options#commands:

    --authrpc.addr value                (default: "localhost")             ($GETH_AUTHRPC_ADDR)
          Listening address for authenticated APIs

    --authrpc.port value                (default: 8551)                    ($GETH_AUTHRPC_PORT)
          Listening port for authenticated APIs

    --authrpc.vhosts value              (default: "localhost")             ($GETH_AUTHRPC_VHOSTS)
          Comma separated list of virtual hostnames from which to accept requests (server
          enforced). Accepts '*' wildcard.
@adlerjohn adlerjohn added the enhancement New feature or request label Oct 9, 2023
@ramin
Copy link
Contributor

ramin commented Oct 9, 2023

we support --rpc.addr and --rpc.port now

      --rpc.addr string                 Set a custom RPC listen address (default: localhost)
      --rpc.port string                 Set a custom RPC port (default: 26658)

But the flag description is incorrect, we actually default to 0.0.0.0 vs localhost/127.0.0.1 the behavior of which is (as your say correctly) to bind the listen address to anywhere.

@ramin ramin added the kind:feat Attached to feature PRs label Oct 9, 2023
@ramin ramin self-assigned this Oct 9, 2023
@ramin ramin changed the title [Feature Request]: Allow binding RPC to only specific addresses (feat): Allow binding RPC to only specific addresses Oct 9, 2023
ramin added a commit that referenced this issue Nov 13, 2023
@ramin
remove unused defaults in gateway/config.go, settle on 127.0.0.1 as d…
79f94e9 
…efault bind address refs #2824 and add some tests
@ramin ramin mentioned this issue Nov 23, 2023
Merged
ramin added a commit that referenced this issue Dec 18, 2023
@ramin
feat(rpc): default binding to localhost vs 0.0.0.0 open to world (#2955)
fae3024 
fixes #2824 which requests we bind to localhost by default (previously
was world accessible)

- sets defaults to bind to localhost properly (previously flags SAID the
defaults were localhost but were actually 0.0.0.0)
- removes an unused `DefaultConfig` defined in `api/gateway/package`
- moves defaults (bind address and port) to vars so we can use them to
set in flag usage messages so they don't drift out of sync with reality
- adds tests for the defaults
- adds test cases for flag definitions
- adds test cases for flag parsing
 
Other

skips suggestion of binding vhosts, we don't do any kind of middleware
checking in rpc (i believe in geth this would be a way to bind to
0.0.0.0 then limit to allowed remote IPs making requests). I'd suggest
this is a more advanced configuration use case and belongs outside of
the node itself to be managed by the infra operator where it'd be
handled by a ingress gateway or routing rule to the
machine/instance/orchestrator where the node software is running
renaynay pushed a commit to renaynay/celestia-node that referenced this issue Jan 15, 2024
@ramin @renaynay
feat(rpc): default binding to localhost vs 0.0.0.0 open to world (cel…
4e1c0a2 
…estiaorg#2955)

fixes celestiaorg#2824 which requests we bind to localhost by default (previously
was world accessible)

- sets defaults to bind to localhost properly (previously flags SAID the
defaults were localhost but were actually 0.0.0.0)
- removes an unused `DefaultConfig` defined in `api/gateway/package`
- moves defaults (bind address and port) to vars so we can use them to
set in flag usage messages so they don't drift out of sync with reality
- adds tests for the defaults
- adds test cases for flag definitions
- adds test cases for flag parsing
 
Other

skips suggestion of binding vhosts, we don't do any kind of middleware
checking in rpc (i believe in geth this would be a way to bind to
0.0.0.0 then limit to allowed remote IPs making requests). I'd suggest
this is a more advanced configuration use case and belongs outside of
the node itself to be managed by the infra operator where it'd be
handled by a ingress gateway or routing rule to the
machine/instance/orchestrator where the node software is running
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ramin ramin

Labels
enhancement New feature or request kind:feat Attached to feature PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(rpc): default binding to localhost vs 0.0.0.0 open to world celestiaorg/celestia-node
2 participants
@ramin @adlerjohn