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

Opampextension - headers in received configs do not correctly marshal into "[REDACTED]" #32983

Closed
adil-sultan opened this issue May 10, 2024 · 4 comments
Labels
bug Something isn't working extension/opamp

Comments

@adil-sultan
Copy link

adil-sultan commented May 10, 2024

Component(s)

extension/opamp

What happened?

Description

When we receive configs via the opamp extension, the header fields aren't being redacted correctly so we receive things like API keys in plain text. We believe this is due to ln272 in opamp_agent.go: yaml.Marshal(o.effectiveConfig.ToStringMap()) where we convert the effectiveConfig.ToStringMap prior to marshaling.

Steps to Reproduce

  1. run the collector via the builder using a config w/ extension configured over WS - can use https://github.com/lightstep/otel-collector-charts/blob/main/example/vm/config.yaml as an example. The received config will contain unredacted Authorization headers

  2. Add a opampextension to the effective.yaml used by opampextension.opamp_agent_test.TestComposeEffectiveConfig. Print the config and you'll see an unredacted Authorization Headers.

Expected Result

authorization:[REDACTED]

Actual Result

authorization: bearer superSecretToken

Collector version

v0.99.0

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

# Last Collector-Contrib Validation: v0.96.0
receivers:
    prometheus/self:
        config:
            scrape_configs:
                - job_name: otel-collector
                  scrape_interval: 5s
                  static_configs:
                      - labels:
                            collector_name: test-collector
                        targets:
                            - 0.0.0.0:8888
    hostmetrics:
        collection_interval: "30s"
        scrapers:
            cpu:
                metrics:
                    system.cpu.utilization:
                        enabled: true
            disk: {}
            load: {}
            filesystem:
                metrics:
                    system.filesystem.utilization:
                        enabled: true
                exclude_mount_points:
                    match_type: regexp
                    mount_points:
                        - /dev/.*
                        - /proc/.*
                        - /sys/.*
                        - /run/k3s/containerd/.*
                        - /var/lib/docker/.*
                        - /var/lib/kubelet/.*
                        - /snap/.*
                exclude_fs_types:
                    match_type: strict
                    fs_types:
                        - autofs
                        - binfmt_misc
                        - bpf
                        - cgroup2
                        - configfs
                        - debugfs
                        - devpts
                        - devtmpfs
                        - fusectl
                        - hugetlbfs
                        - iso9660
                        - mqueue
                        - nsfs
                        - overlay
                        - proc
                        - procfs
                        - pstore
                        - rpc_pipefs
                        - securityfs
                        - selinuxfs
                        - squashfs
                        - sysfs
                        - tracefs
            memory:
                metrics:
                    system.memory.utilization:
                        enabled: true
            network: {}
    otlp:
        protocols:
            grpc:
                endpoint:
            http:
                endpoint:
processors:
    batch:
        send_batch_size: 1000
        timeout: 1s
        send_batch_max_size: 1500
    attributes:
        actions:
            - key: service.version
              action: insert
              from_attribute: service_version
            - key: service.name
              action: upsert
              from_attribute: service_name
    resourcedetection/env:
        detectors: [ env ]
        timeout: 2s
        override: false

exporters:
    otlp/ls:
        endpoint: ingest.test.com:443 # US data center
        headers:
            sample-access-token: "${SAMPLE_ACCESS_TOKEN}"
        timeout: 5s
        sending_queue:
            enabled: true
            num_consumers: 4
            queue_size: 100
        retry_on_failure:
            enabled: true
            max_elapsed_time: 30s
    debug:
        verbosity: basic
        sampling_initial: 5
        sampling_thereafter: 200

extensions:
    health_check:
    opamp:
        server:
            ws:
                endpoint: "wss://opamp.test.com/v1/opamp"
                headers:
                    "Authorization": "bearer ${Super_Secret_Token}"

service:
    extensions: [health_check, opamp]
    telemetry:
        metrics:
            address: :8888
    pipelines:
        traces:
            receivers: [otlp]
            processors: [resourcedetection/env, attributes, batch]
            exporters: [debug, otlp/ls]
        metrics:
            receivers: [otlp, prometheus/self, hostmetrics]
            processors: [resourcedetection/env, attributes, batch]
            exporters: [debug, otlp/ls]
        logs:
            exporters: [debug, otlp/ls]
            processors: [resourcedetection/env, attributes, batch]
            receivers: [otlp] # update with your receiver name

Log output

No response

Additional context

No response

@adil-sultan adil-sultan added bug Something isn't working needs triage New item requiring triage labels May 10, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@evan-bradley evan-bradley removed the needs triage New item requiring triage label May 10, 2024
@evan-bradley
Copy link
Contributor

Hi @adil-sultan, thanks for reporting this. This is a known problem with how the Collector APIs unmarshal the config into a confmap.Conf. I am working through fixing this right now.

@adil-sultan
Copy link
Author

@evan-bradley That's great to hear. If there's a PR I can follow along with that would be much appreciated. Thanks!

evan-bradley added a commit that referenced this issue Jun 3, 2024
**Description:** <Describe what has changed.>

Redacts primitive values in the effective config reported by the
extension.

Necessary for
#31641
until
open-telemetry/opentelemetry-collector#10139 is
resolved.

Relates to
#32983
@evan-bradley
Copy link
Contributor

I'm closing this as solved via #32983. Right now we redact almost all values in the config, but we will work toward exposing non-sensitive values as we make progress in the Collector APIs or offer settings through the extension.

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

No branches or pull requests

2 participants