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

Suppress warning about 0.0.0.0 #11713

Closed
yurishkuro opened this issue Nov 19, 2024 · 4 comments · Fixed by #11902
Closed

Suppress warning about 0.0.0.0 #11713

yurishkuro opened this issue Nov 19, 2024 · 4 comments · Fixed by #11902

Comments

@yurishkuro
Copy link
Member

Is your feature request related to a problem? Please describe.

Using the 0.0.0.0 address exposes this server to every network interface, 
which may facilitate Denial of Service attacks.

When running inside a container, binaries are expected to listen to 0.0.0.0, because they don't know any other IP and localhost makes them inaccessible in many scenarios.

Describe the solution you'd like

I don't want this warning to appear when the 0.0.0.0 configuration is actually valid and expected.

I propose adding some environment variable that can be set in the Dockerfile that will instruct the Collector not to log this warning.

Describe alternatives you've considered

To my knowledge there is no reliable way to determine if a binary is running inside a container.

Related issues

@mx-psi
Copy link
Member

mx-psi commented Nov 20, 2024

Based on #8510 (comment) we should remove the warning. Do you want to file a PR for this?

@yurishkuro
Copy link
Member Author

I am not opposed to the warning when it is actually relevant. For example, Jaeger all-in-one comes with a built-in configuration (not provided by user) and we have to hardcode 0.0.0.0 IP there to make it work in containers. But if the raw binary is run and also listens on 0.0.0.0 then it's back to the same CVE, so a warning to the user is good. Not to mention in situations where IP is actually provided by the user and they make a mistake which makes them insecure.

@mx-psi
Copy link
Member

mx-psi commented Nov 20, 2024

The warning will remain in our security documentation, but based on the vote we did it should be removed. I think regardless, it is quite difficult to reliably figure out if you are running inside a container or not, so I am not sure how we would go about this if we wanted to keep the warning

@yurishkuro
Copy link
Member Author

That's why I am proposing a well-defined env var like OTEL_IN_CONTAINER that can be used. We could even let the collector use this in place of the former feature flag to allow it to automatically decide which IP is the most appropriate to use, so that when a user just says "I want grpc receiver" they don't need to fiddle with the endpoint, collector will do the right thing.

yurishkuro added a commit to jaegertracing/jaeger that referenced this issue Nov 21, 2024
## Which problem is this PR solving?
- Resolves #6209
- In #6226 we changed the all-in-one config to always use 0.0.0.0, which
is not a secure option when running the binary directly on host (vs.
running in container)

## Description of the changes
- Introduce `JAEGER_LISTEN_HOST` env var used from all-in-one/v2 config
- Default it to `localhost` suitable for running directly on host
- Override it to 0.0.0.0 in the Dockerfile for v2

## How was this change tested?
- ran the binary without env var, no security warnings
- ran with JAEGER_LISTEN_HOST=0.0.0.0 - warnings are displayed as
expected (pending
open-telemetry/opentelemetry-collector#11713)

Signed-off-by: Yuri Shkuro <github@ysh.us>
github-merge-queue bot pushed a commit that referenced this issue Dec 17, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Based on the discussions from
#11713 (comment)
and
#8510 (comment)
the warning message logged when `0.0.0.0` is used, should be removed.

<!-- Issue number if applicable -->
#### Link to tracking issue
Probably fixes
#11713

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

/cc @mx-psi

---------

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this issue Dec 19, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Based on the discussions from
open-telemetry#11713 (comment)
and
open-telemetry#8510 (comment)
the warning message logged when `0.0.0.0` is used, should be removed.

<!-- Issue number if applicable -->
#### Link to tracking issue
Probably fixes
open-telemetry#11713

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

/cc @mx-psi

---------

Signed-off-by: ChrsMark <chrismarkou92@gmail.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.

2 participants