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

Explicitly set endpoint host to 0.0.0.0 in all Collector examples #3839

Closed
mx-psi opened this issue Jan 22, 2024 · 6 comments · Fixed by #3847
Closed

Explicitly set endpoint host to 0.0.0.0 in all Collector examples #3839

mx-psi opened this issue Jan 22, 2024 · 6 comments · Fixed by #3847
Assignees
Labels
bug Something isn't working sig:collector

Comments

@mx-psi
Copy link
Member

mx-psi commented Jan 22, 2024

What needs to be changed?

Per the poll at open-telemetry/opentelemetry-collector#8510 (comment), the Collector will eventually change the default endpoint host in all servers from 0.0.0.0 to localhost. Before this happens, I would like to update all examples in opentelemetry.io to explicitly set the endpoint to 0.0.0.0 to minimize disruption to end users as much as possible.

The most common issue is on the OTLP receiver examples, where we need to change examples like:

receivers:
  otlp:
    protocols:
      grpc:

to this:

receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317

This also needs to be done for the HTTP endpoint, as well as other components mentioned in open-telemetry/opentelemetry-collector-contrib/issues/30702.

What is the name + path of the page that needs changed?

There are a lot of pages that need to be updated. For example, you can find many instances on https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry.io+otlp%3A&type=code (some false positives as well).

I think the other components that may need updates are

I may be missing some.

Additional context: We don't have a explicitly deadline for when the change will happen, we will likely leave some time for this

@mx-psi mx-psi added bug Something isn't working sig:collector labels Jan 22, 2024
@svrnm
Copy link
Member

svrnm commented Jan 22, 2024

Should we add a comment above each line similar to what we have for debugg/logging exporter, e.g.

receivers:
  otlp:
    protocols:
      grpc:
        # Note that the default endpoint is localhost:4317, if you ...
		endpoint: 0.0.0.0:4317

@mx-psi
Copy link
Member Author

mx-psi commented Jan 22, 2024

We are not changing the default endpoint yet, but I think it makes sense to add a comment. Maybe we can link to https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks ?

@svrnm
Copy link
Member

svrnm commented Jan 22, 2024

We are not changing the default endpoint yet, but I think it makes sense to add a comment. Maybe we can link to open-telemetry/opentelemetry-collector@main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks ?

Yeah, or have both and one commented out, etc. Let's have PR against it and we can take a look what looks best

@theletterf theletterf self-assigned this Jan 23, 2024
@theletterf
Copy link
Member

theletterf commented Jan 23, 2024

Preparing a PR for this.

@mx-psi Does this apply to all receivers that expose loopback+port, like the Prometheus receiver on port 8888 for example?

Edit: Some extensions also seem to use token URLs and similar with localhost...

@theletterf
Copy link
Member

@mx-psi Second question: do we need to also edit this in the language instrumentation examples? Example:

OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318

@mx-psi
Copy link
Member Author

mx-psi commented Jan 23, 2024

Does this apply to all receivers that expose loopback+port, like the Prometheus receiver on port 8888 for example?

The rationale is https://cwe.mitre.org/data/definitions/1327.html, the original motivation to care about this was open-telemetry/opentelemetry-collector/issues/6151 (i.e. to mitigate the effect of CVEs such as GHSA-69cg-p879-7622). So it applies to all servers exposed by the Collector: if we can make them use localhost, we should.

Second question: do we need to also edit this in the language instrumentation examples? Example:

AIUI this is not affected by the CWE and would not need a change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sig:collector
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants