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

Environment variables for OTLP client TLS key / certificate #1363

Closed
anuraaga opened this issue Jan 22, 2021 · 3 comments
Closed

Environment variables for OTLP client TLS key / certificate #1363

anuraaga opened this issue Jan 22, 2021 · 3 comments
Labels
area:sdk Related to the SDK priority:p3 Lowest priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:miscellaneous For issues that don't match any other spec label

Comments

@anuraaga
Copy link
Contributor

What are you trying to achieve?

Ability to set client key / certificate for mTLS.

Additional context.

https:/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/exporter.md#configuration-options

We currently only have a CERTIFICATE_FILE environment variable, which is a bit misleading since this only refers to trusted certificates. For mTLS, there would need to be two additional variables, one for the client certificate and one for the client private key. I think we should have three variables

OTEL_EXPORTER_OTLP_TLS_PRIVATE_KEY
OTEL_EXPORTER_OTLP_TLS_CERTIFICATE
OTEL_EXPORTER_OTLP_TLS_TRUSTED_CERTIFICATE

Reference: open-telemetry/opentelemetry-java-instrumentation#1829
@anuraaga anuraaga added the spec:miscellaneous For issues that don't match any other spec label label Jan 22, 2021
@anuraaga
Copy link
Contributor Author

anuraaga commented Jan 22, 2021 •
edited
Loading

Well - looking at the documentation for OTLP_EXPORTER_OTLP_CERTIFICATE it seems to be indicating this is a client certificate, not a trusted one Path to certificate file for TLS credentials of gRPC client. Should only be used for a secure connection.- note "TLS credentials of gRPC client", but these are credentials of the server. But this can't be true since there would need to be another variable corresponding to the private key. So not even really sure how to implement that variable.

@anuraaga anuraaga mentioned this issue Jan 22, 2021
Closed
@andrewhsu andrewhsu added priority:p3 Lowest priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs area:sdk Related to the SDK labels Jan 22, 2021
@anuraaga
Copy link
Contributor Author

I'm a bit worried the current environment variable is misspecified so I think at least confirming that before GA would be good I think. Might be good to remove it for now if not adding all three

/cc @tedsuo

@anuraaga anuraaga mentioned this issue Jan 26, 2021
Closed
codeboten pushed a commit to codeboten/opentelemetry-specification that referenced this issue Jul 7, 2021
clarify meaning of "Certificate File"
bf26172 
The current version of the specification makes me think that Certificate File is intended to be used as part of the client authentication, which it is not. This change addresses part of the confusion discussed in open-telemetry#1363, the certificate file option and environment variable points to the certificate used to verify the server's certificate. 

See also open-telemetry#1375.
@codeboten codeboten mentioned this issue Jul 7, 2021
Merged
SergeyKanzhelev added a commit that referenced this issue Jul 7, 2021
@SergeyKanzhelev
clarify meaning of Certificate File (#1803)
2e7d017 
* clarify meaning of "Certificate File" 

The current version of the specification makes me think that Certificate File is intended to be used as part of the client authentication, which it is not. This change addresses part of the confusion discussed in #1363, the certificate file option and environment variable points to the certificate used to verify the server's certificate. 

See also #1375.

* update changelog

* Update CHANGELOG.md

Co-authored-by: Sergey Kanzhelev <[email protected]>
This was referenced Feb 9, 2022
Closed
Closed
@jack-berg
Copy link
Member

Resolved in #2370.

tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-proto that referenced this issue Apr 20, 2023
@SergeyKanzhelev @tigrannajaryan
clarify meaning of Certificate File (#1803)
4c75ee6 
* clarify meaning of "Certificate File"

The current version of the specification makes me think that Certificate File is intended to be used as part of the client authentication, which it is not. This change addresses part of the confusion discussed in open-telemetry/opentelemetry-specification#1363, the certificate file option and environment variable points to the certificate used to verify the server's certificate.

See also open-telemetry/opentelemetry-specification#1375.

* update changelog

* Update CHANGELOG.md

Co-authored-by: Sergey Kanzhelev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:sdk Related to the SDK priority:p3 Lowest priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:miscellaneous For issues that don't match any other spec label
Projects
None yet
Milestone
No milestone
3 participants
@andrewhsu @anuraaga @jack-berg