search

ethersphere / bee

Bee is a Swarm client implemented in Go. It’s the basic building block for the Swarm network: a private; decentralized; and self-sustaining network for permissionless publishing and access to your (application) data.
https://www.ethswarm.org
BSD 3-Clause "New" or "Revised" License
1.44k stars 338 forks source link

feat: remove debug api #4674

Closed acha-bill closed 1 month ago

acha-bill commented 1 month ago

Checklist

Description

Remove debug API

To keep this PR concise, the --restricted option will be removed in another PR.

Part one of #4669

acha-bill commented 1 month ago

@janos for the first reference, it seems the option here is not related to bee itself but to the HTTP transport used during tests. See https://github.com/ethersphere/bee/blob/master/pkg/api/api_test.go#L268

That's why I left it. Do you think we should remove this logic?

janos commented 1 month ago

@janos for the first reference, it seems the option here is not related to bee itself but to the HTTP transport used during tests. See https://github.com/ethersphere/bee/blob/master/pkg/api/api_test.go#L268

That's why I left it. Do you think we should remove this logic?

It looks to me that the transport change does nothing to do with the DebugAPI anymore as the condition for mounting routes is removed. At least, the field name should be renamed. The change to the httpClient if o.DebugAPI is false is related to the subdomain support if the DebugAPI is not set to always dial to the server's host and port. It may be the case that the testing will work regardless if that logic is constant.

istae commented 1 month ago

LGTM.

I must say that having one api to serve both business, infrastructure and observability information creates a risk that some of endpoints can be exposed by the user without the user being aware. The purpose of the debug api was to protect infrastructure, observability and possible other sensitive endpoints on the networking layer instead application layer or even on proxy application if the user exposes the bee api, by having the debug api tcp listener on the loopback ip by default. It would require the user to override the listener ip address in order to expose it. It is required for user to be well aware and educated of specific endpoints in order to protect its bee node if bee api is exposed to the internet.

I think that only the p2p port should be exposed. Having multiple ports gives the impression that it is ok to expose one and protect the other. By forcing the user to use a single port for the API, they have to make a conscious choise which endpoints they want to expose using some external later, eg nginx proxy.

janos commented 1 month ago

LGTM. I must say that having one api to serve both business, infrastructure and observability information creates a risk that some of endpoints can be exposed by the user without the user being aware. The purpose of the debug api was to protect infrastructure, observability and possible other sensitive endpoints on the networking layer instead application layer or even on proxy application if the user exposes the bee api, by having the debug api tcp listener on the loopback ip by default. It would require the user to override the listener ip address in order to expose it. It is required for user to be well aware and educated of specific endpoints in order to protect its bee node if bee api is exposed to the internet.

I think that only the p2p port should be exposed. Having multiple ports gives the impression that it is ok to expose one and protect the other. By forcing the user to use a single port for the API, they have to make a conscious choise which endpoints they want to expose using some external later, eg nginx proxy.

Exactly, the conscious choice to expose the API (either over proxy or just setting the listening ip to 0 or exposed IP), but without exposing the internal endpoints like /debug or /metrics by protecting them on the networking layer (listening ip) instead requiring explicit protection on the proxy where user has to know the complete list of endpoints that must be blocked. The user always needs to keep track on upgrades to make sure that some newly added endpoint has to be blocked leaving the possibility for the exploits.

In any case, the decision has been made to remove the debug api long time ago by team members that are not longer with swarm, I just want to raise the awareness of tradeoffs that such decision brings.