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

Flow mode component otelcol.vsphere.receiver does use most configured TLS settings #193

Open
geoffmore opened this issue Apr 5, 2024 · 7 comments
Labels
bug Something isn't working needs-attention

Comments

@geoffmore
Copy link

geoffmore commented Apr 5, 2024

What's wrong?

I get an error for an untrusted CA even when setting tls.insecure_skip_verify to true.

I wrote a test in vcenter_test.go TestArguments_UnmarshalRiver to make sure the river config for the TLS block was being unmarshalled correctly in vcenter_test.go. The test adds

tls {
    insecure_skip_verify = true
}

right under the collection_interval line.

The test is

require.Equal(t, true, otelArgs.TLSClientSetting.InsecureSkipVerify)

and returned the expected true and the test passed.

In my testing, I noticed that the insecure option is respected.

Looking at code, it appears that only the insecure option is respected because of the underlying client. Given the limited function arguments relative to the tls block of the otelcol.receiver.vcenter, I believe another client should be used.

Steps to reproduce

  1. Add an argument other than insecure to the tls block of component otelcol.receiver.vcenter
  2. See error logs if the CA/Cert is not trusted

System information

Fedora 6.7.11-200.fc39.x86_64

Software version

Grafana Agent v0.40.3

Configuration

otelcol.receiver.vcenter "default" {
  endpoint = "https://URL"
  username = "Username"
  password = "Password"

  tls {
    insecure_skip_verify = true
    insecure = false // testing true
    ca_file = "/etc/agent/ca.pem"
    cert_file = "/etc/agent/cert.pem"
    server_name = "URL"
  }
  debug_metrics {}

  output {
    metrics = [otelcol.processor.batch.default.input]
  }
}

otelcol.processor.batch "default" {
  output {
    metrics = [otelcol.exporter.otlp.default.input]
  }
}

otelcol.auth.basic "grafana_cloud_tempo" {
    username = "temp-user"
    password = "tempo-password"
}

otelcol.exporter.otlp "default" {
  client {
    endpoint = "tempo-endpoint"
    auth = otelcol.auth.basic.grafana_cloud_tempo.handler
  }
  debug_metrics {}
}

Logs

unable to establish a connection to the vSphere SDK unable to connect to vSphere SDK on listed endpoint: Post \"https://<vsphere_url>/sdk\": tls: failed to verify certificate: x509: certificate signed by unknown authority
@geoffmore geoffmore added the bug Something isn't working label Apr 5, 2024
@hainenber
Copy link
Contributor

I think the config's logic makes sense here: if insecure is set as false, it'll force the verification and supersedes other TLS options. Might be the doc can be clearer regarding this.

@geoffmore
Copy link
Author

Based on the tls block, I would expect the insecure option to disable TLS entirely, which is very different than accepting a cert with an untrusted CA. Having the options present in different places, but unused seems strange to me.

Either way, this feels like an issue that can/should be resolved in the upstream https://github.com/open-telemetry/opentelemetry-collector-contrib/, since that repo has the code that the Grafana agent uses here.

I have ideas for a way to enable all config options, but it will probably be easier to just create an PR to describe what I'm aiming for.

What are your thoughts on being able to leverage all of the options of the tls block?

@hainenber
Copy link
Contributor

hainenber commented Apr 6, 2024

I do agree on resolving this on upstream, tbh. We're bound by the exposed interface here.

What are your thoughts on being able to leverage all of the options of the tls block?

I'm all for it :D

@rfratto
Copy link
Member

rfratto commented Apr 11, 2024

Hi there 👋

On April 9, 2024, Grafana Labs announced Grafana Alloy, the spirital successor to Grafana Agent and the final form of Grafana Agent flow mode. As a result, Grafana Agent has been deprecated and will only be receiving bug and security fixes until its end-of-life around November 1, 2025.

To make things easier for maintainers, we're in the process of migrating all issues tagged variant/flow to the Grafana Alloy repository to have a single home for tracking issues. This issue is likely something we'll want to address in both Grafana Alloy and Grafana Agent, so just because it's being moved doesn't mean we won't address the issue in Grafana Agent :)

@rfratto rfratto transferred this issue from grafana/agent Apr 11, 2024
Copy link
Contributor

This issue has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If the opened issue is a bug, check to see if a newer release fixed your issue. If it is no longer relevant, please feel free to close this issue.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your issue will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

@bmhkb4
Copy link

bmhkb4 commented Nov 12, 2024

I am seeing this same thing on the latest alloy client. No configurable TLS options work except insecure = true. Not even providing the ca_pem, ca_file fixes this issue.

@dehaansa
Copy link
Contributor

While we look at this issue in the upstream collector, there is a workaround that can help in certain environments.

The internal golang certificate pool code contains this comment. At least one user was able to use the SSL_CERT_DIR environment variable to load in the certificate files.

// SystemCertPool returns a copy of the system cert pool.
//
// On Unix systems other than macOS the environment variables SSL_CERT_FILE and
// SSL_CERT_DIR can be used to override the system default locations for the SSL
// certificate file and SSL certificate files directory, respectively. The
// latter can be a colon-separated list.

djaglowski added a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Dec 3, 2024
…ent call (#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The `govmomi` client used in the receiver attempts to validate the
connection to vcenter before the existing code sets the TLS options
(other than insecure) in the client. This is a limitation of the
`govmomi` wrapper, as discussed on this issue:
vmware/govmomi#1200 .

<!-- Issue number (e.g. #1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

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

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
shivanthzen pushed a commit to shivanthzen/opentelemetry-collector-contrib that referenced this issue Dec 5, 2024
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The `govmomi` client used in the receiver attempts to validate the
connection to vcenter before the existing code sets the TLS options
(other than insecure) in the client. This is a limitation of the
`govmomi` wrapper, as discussed on this issue:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

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

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
ZenoCC-Peng pushed a commit to ZenoCC-Peng/opentelemetry-collector-contrib that referenced this issue Dec 6, 2024
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The `govmomi` client used in the receiver attempts to validate the
connection to vcenter before the existing code sets the TLS options
(other than insecure) in the client. This is a limitation of the
`govmomi` wrapper, as discussed on this issue:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

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

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this issue Dec 17, 2024
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The `govmomi` client used in the receiver attempts to validate the
connection to vcenter before the existing code sets the TLS options
(other than insecure) in the client. This is a limitation of the
`govmomi` wrapper, as discussed on this issue:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

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

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
AkhigbeEromo pushed a commit to sematext/opentelemetry-collector-contrib that referenced this issue Jan 13, 2025
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The `govmomi` client used in the receiver attempts to validate the
connection to vcenter before the existing code sets the TLS options
(other than insecure) in the client. This is a limitation of the
`govmomi` wrapper, as discussed on this issue:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

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

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-attention
Projects
None yet
Development

No branches or pull requests

5 participants