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

Default TLS credentials using system certificates for OTLP GRPC #1584

Closed
vmihailenco opened this issue Feb 25, 2021 · 8 comments · Fixed by #2432
Closed

Default TLS credentials using system certificates for OTLP GRPC #1584

vmihailenco opened this issue Feb 25, 2021 · 8 comments · Fixed by #2432
Labels
pkg:exporter:otlp Related to the OTLP exporter package

Comments

@vmihailenco
Copy link
Contributor

What do you think about using system certificates for GRPC OTLP exporter by default? E.g. now I have to

	certPool, err := x509.SystemCertPool()
	if err != nil {
		return nil, err
	}
	creds := credentials.NewClientTLSFromCert(certPool, "")

	driver := otlpgrpc.NewDriver(
		otlpgrpc.WithEndpoint("some.domain.com:4317"),
		otlpgrpc.WithTLSCredentials(creds),
	)

But I would expect GRPC to use system certificate by default. And otlpgrpc.WithInsecure can be used to disable TLS. I can send a PR if this is accepted.

@seh
Copy link
Contributor

seh commented Feb 25, 2021

Note that if you pass nil as the first argument to credentials.NewClientTLSFromCert, you get the system certificate pool automatically. That's per the documentation for tls.Config's "RootCAs" field:

RootCAs defines the set of root certificate authorities that clients use when verifying server certificates. If RootCAs is nil, TLS uses the host's root CA set.

@MrAlias
Copy link
Contributor

MrAlias commented Feb 25, 2021

From the SIG meeting: we would like to have this fixed for the RC, but it is something we can add after with backwards compatible support.

@MrAlias
Copy link
Contributor

MrAlias commented Feb 25, 2021

A possible first step might be to update the documentation to at least describe the solution here.

@vmihailenco
Copy link
Contributor Author

@MrAlias should system certificates be the default or there is some other plan how this should be fixed? It is not obvious what was decided...

@MrAlias
Copy link
Contributor

MrAlias commented Feb 26, 2021

@vmihailenco We talked more about priority of getting this in for the RC, but I think there was a tacit agreement from the SIG meeting yesterday that this having the system certificates be the default makes sense.

We would want to be sure to still provide the ability to override this by setting the credentials or setting insecure. @seh mentioned that it is an error to set both credentials and insecure. We should add a test to accompany the change to ensure we support all user situations.

@seh
Copy link
Contributor

seh commented Feb 26, 2021

@seh mentioned that it is an error to set both credentials and insecure. We should add a test to accompany the change to ensure we support all user situations.

Slight correction: It's an error to set neither option as well. The gRPC client requires that the caller tell it either to behave insecurely, or to use a specific set of credentials (which in this case really means which CAs to trust as issuers for the server's X.509 certificate, with the client not presenting a certificate of its own to the server).

@vmihailenco
Copy link
Contributor Author

vmihailenco commented Feb 27, 2021

It's an error to set neither option as well.

This is the current behavior. Looks like there is nothing to change after all...

The gRPC client requires that the caller tell it either to behave insecurely, or to use a specific set of credentials

This is true and this is what I suggested to change for OTLP exporter.

PS @seh I am confused whether you are just describing current situation (i.e. GRPC client behavior) or describing how OTLP should work in the end...

@kvrhdn
Copy link
Contributor

kvrhdn commented Nov 24, 2021

Hi, I'd like to bump this issue. I believe the problem I'm facing is related.

I want to use environment variables to control where my app sends traces: if I'm working locally this would be a local stack running on http://localhost:4317, if I'm working with my cloud solution I want it to be the endpoint of my backend.

Unfortunately I can't switch between them without doing a code change because I need to set WithTLSCredentials.

Config for local stack

Environment variables:

OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317

Code needed to setup the exporter is nice and tidy:

exporter, err := otlptracegrpc.New(ctx)

Config for remote endpoint with TLS

Environment variables:

OTEL_EXPORTER_OTLP_ENDPOINT=tempo-us-central-0.grafana.net:443
OTEL_EXPORTER_OTLP_HEADERS=authorization=Basic ...

If I don't change the code I get:

OTel error: traces exporter is disconnected from the server tempo-us-central-0.grafana.net:443: grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)

Correct code:

exporter, err := otlptracegrpc.New(
    ctx,
    otlptracegrpc.WithTLSCredentials(credentials.NewClientTLSFromCert(nil, ""))
)

The only place I found this solution btw is in the Honeycomb docs: https://docs.honeycomb.io/getting-data-in/go/opentelemetry/#installation-and-exporter-configuration

Solution

I can see two solutions that would work for me:

  • if the endpoint is secure, set otlptracegrpc.WithTLSCredentials(credentials.NewClientTLSFromCert(nil, "")) by default
  • if the endpoint is insecure, ignore otlptracegrpc.WithTLSCredentials()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:exporter:otlp Related to the OTLP exporter package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants