[all][meta] Make existing sensitive configuration fields opaque #17273

mx-psi · 2022-12-27T16:24:08Z

Overview

As part of open-telemetry/opentelemetry-collector#6851 and with the goal of creating a system to query the Collector's configuration, the configopaque.String type alias has been added to the core Collector library to be used on fields that contain sensitive information.

To ensure that no sensitive information is leaked on existing components' configuration, we need to audit their configuration schema and change the type on sensitive fields to use configopaque.String.

This issue intends to list all components where such a change is needed.

To generate the initial list, I searched for instances of "Token", "Key", "Password" and "Secret" on any file named config.go on this repository.

How to make the change

Changing a field type is a breaking change and should be noted as such on the changelog. Codeowners of a given component can choose to make this change with or without a deprecation, depending on how many users a component has as a Go module:

Without deprecation by just changing the type to configopaque.String and noting it as a breaking change in the changelog (see [confighttp] Change Headers field type to have opaque values opentelemetry-collector#6637 for an example)
With deprecation by creating a temporary field and deprecating the current field (see Add configopaque package opentelemetry-collector#6470 (comment) for a more detailed step by step plan)

My expectation is that for most fields we can do this without deprecation since usage of the Go API is minimal/nonexistent.

List of subtasks

Receivers

Processors

[processor/resourcedetection] Use configopaque for consul token field #17314

Exporters

Extensions

[extension/asapauth] Use configopaque for private_key field
[extension/basicauth] Use configopaque for client_auth::password field
[extension/bearertokenauth] Use configopaque for token field
[extension/oauth2clientauth] Use configopaque for client_secret field

The text was updated successfully, but these errors were encountered:

mx-psi · 2022-12-28T10:28:17Z

This issue is big, so if you want to work on it, please pick a few components at a time and list them in a comment so that others can also work on it, thanks!

dyladan · 2022-12-28T17:13:57Z

i'll do the dynatrace one

atoulme · 2022-12-28T17:22:21Z

I’ll do the splunkhec and signalfx ones. Humio is deprecated, maybe we can pass on it.

mx-psi · 2022-12-29T09:06:53Z

@atoulme @dyladan I created child issues for those components and assigned them to you. I also marked humio as done since it's deprecated (thanks!)

dyladan · 2022-12-30T18:23:51Z

Looks like dynatrace was already done by @codeboten in 62da158 #17077

gbbr · 2023-01-02T09:12:23Z

Did processor/resourcedetectionprocessor: #17314 / #17315

gbbr · 2023-01-02T09:27:37Z

Did extension/asapauthextension: #17316 / #17317

gbbr · 2023-01-02T09:41:58Z

Did the rest of the extensions here: #17318. I hope it's ok for them to be part of the same PR. It was becoming tedious to do them individually.

gbbr · 2023-01-03T09:34:27Z

Migrated all receivers in #17353

gbbr · 2023-01-03T09:56:26Z

...and the exporters are here: #17354.

I think that should have covered everything now.

…String This change migrates the remaining exporters from open-telemetry#17273 to use configopaque.String: - [x] [exporter/alibabacloudlogservice] Use configopaque for access_key_secret field - [x] [exporter/azuredataexplorer] Use configopaque for application_key field - [x] [exporter/azuremonitor] Use configopaque for instrumentation_key field - [x] [exporter/coralogix] Use configopaque for private_key field - [x] [exporter/elasticsearch] Use configopaque for api_key and password fields - [x] [exporter/influxdb] Use configopaque for token and password fields - [x] [exporter/instana] Use configopaque for agent_key field - [x] [exporter/logicmonitor] Use configopaque for apitoken::access_key fields - [x] [exporter/logzio] Use configopaque for account_token field - [x] [exporter/mezmo] Use configopaque for ingest_key field - [x] [exporter/pulsar] Use configopaque for auth::Token::Token and auth::athenz::private_key fields - [x] [exporter/sapm] Use configopaque for access_token field - [x] [exporter/tencentcloudlogservice] Use configopaque for secret_key field

This change updates sensitive information described in open-telemetry#17273 for all receivers: - [x] [receiver/aerospike] Use configopaque for password field - [x] [receiver/awsfirehose] Use configopaque for access_key field - [x] [receiver/bigip] Use configopaque for password field - [x] [receiver/cloudfoundry] Use configopaque for password field - [x] [receiver/couchdb] Use configopaque for password field - [x] [receiver/elasticsearch] Use configopaque for password field - [x] [receiver/jmx] Use configopaque for password, keystore_password, truststore_password fields - [x] [receiver/mongodbatlas] Use configopaque for private_key and secret fields - [x] [receiver/mongodb] Use configopaque for password field - [x] [receiver/mysql] Use configopaque for password field - [x] [receiver/nsxt] Use configopaque for password field - [x] [receiver/podman] Use configopaque for ssh_passphrase field - [x] [receiver/postgresql] Use configopaque for password field - [x] [receiver/pulsar] Use configopaque for auth::Token::Token and auth::athenz::private_key fields - [x] [receiver/rabbitmq] Use configopaque for password field - [x] [receiver/redis] Use configopaque for password field - [x] [receiver/riak] Use configopaque for password field - [x] [receiver/saphana] Use configopaque for password field - [x] [receiver/snmp] Use configopaque for auth_password and privacy_password fields - [x] [receiver/snowflake] Use configopaque for password field - [x] [receiver/solace] Use configopaque for password field - [x] [receiver/vcenter] Use configopaque for password field

Updates open-telemetry#17273

MovieStoreGuy · 2023-01-09T01:25:01Z

Hey @mx-psi ,

After reviewing #17354, I am not certain that this would be considered a breaking change considering from the user's perspective nothing has changed.
From a development perspective, I don't think this is a breaking change either considering that it is possible to cast back to the original string with no impact.

Is there anything I am missing that would cause this to be a breaking change?

mx-psi · 2023-01-10T10:24:29Z

Is there anything I am missing that would cause this to be a breaking change?

This will not affect end-users of Collector binaries built using the Collector builder (either official or custom), but there exist programs written before changing the type that will fail to build after changing the type because of a type mismatch. This is a breaking change, and semi-official Go tooling like apidiff would identify it as such.

I agree it is not a breaking change that would cause much pain (it's Go API only, and easy to fix), which is why I argue above that it's fine to do this in one step without deprecation, but (unless this causes significant confusion), I don't see why we should not label it as breaking when it is.

…String (#17354) This change migrates the remaining exporters from #17273 to use configopaque.String: - [x] [exporter/alibabacloudlogservice] Use configopaque for access_key_secret field - [x] [exporter/azuredataexplorer] Use configopaque for application_key field - [x] [exporter/azuremonitor] Use configopaque for instrumentation_key field - [x] [exporter/coralogix] Use configopaque for private_key field - [x] [exporter/elasticsearch] Use configopaque for api_key and password fields - [x] [exporter/influxdb] Use configopaque for token and password fields - [x] [exporter/instana] Use configopaque for agent_key field - [x] [exporter/logicmonitor] Use configopaque for apitoken::access_key fields - [x] [exporter/logzio] Use configopaque for account_token field - [x] [exporter/mezmo] Use configopaque for ingest_key field - [x] [exporter/pulsar] Use configopaque for auth::Token::Token and auth::athenz::private_key fields - [x] [exporter/sapm] Use configopaque for access_token field - [x] [exporter/tencentcloudlogservice] Use configopaque for secret_key field

…que.String` **Description:** Split out from: open-telemetry#17353 **Link to tracking Issue:** open-telemetry#17273

…opaque.String` **Description:** Split out from: open-telemetry#17353 **Link to tracking Issue:** open-telemetry#17273

…Password}` fields to be of `configopaque.String` **Description:** Split out from: open-telemetry#17353 **Link to tracking Issue:** open-telemetry#17273

…igopaque.String` **Description:** Split out from: open-telemetry#17353 **Link to tracking Issue:** open-telemetry#17273

… to be `configopaque.String` **Description:** Split out from: open-telemetry#17353 **Link to tracking Issue:** open-telemetry#17273

…opaque.String` **Description:** Split out from: open-telemetry#17353 **Link to tracking Issue:** open-telemetry#17273

…e `configopaque.String` (#23832) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…opaque.String` (#24063) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…Password}` fields to be of `configopaque.String` (#24064) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…igopaque.String` (#24065) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…opaque.String` (#24067) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…,TruststorePassword}` to be `configopaque.String` (#23864) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…que.String` (#24062) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

…aque.String` (#24060) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273 Co-authored-by: Alex Boten <aboten@lightstep.com>

…onfigopaque.String` (#23829) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

mackjmr · 2023-07-20T16:43:57Z

All the receivers have been updated @mx-psi, I think this issue is good to close!

mx-psi · 2023-07-21T09:29:20Z

Thanks @mackjmr!! 🚀 🚀

mx-psi added enhancement New feature or request help wanted Extra attention is needed labels Dec 27, 2022

mx-psi mentioned this issue Dec 27, 2022

[config] configopaque adoption open-telemetry/opentelemetry-collector#6851

Closed

6 tasks

This comment was marked as resolved.

Sign in to view

mx-psi changed the title ~~[WIP][all][meta] Make existing sensitive configuration fields opaque~~ [all][meta] Make existing sensitive configuration fields opaque Dec 28, 2022

mx-psi added the good first issue Good for newcomers label Dec 28, 2022

gbbr mentioned this issue Jan 2, 2023

[processor/resourcedetection] Use configopaque for consul token field #17314

Closed

gbbr mentioned this issue Jan 2, 2023

[extension/asapauth] Use configopaque for private_key field #17316

Closed

gbbr mentioned this issue Jan 2, 2023

extension/{basicauth,bearertoken,oauth2client}: use configopaque.String #17318

Merged

gbbr mentioned this issue Jan 3, 2023

receiver: migrate to configopaque.String #17353

Closed

22 tasks

gbbr mentioned this issue Jan 3, 2023

exporters: migrate remaining sensitive config fields to configopaque.String #17354

Merged

13 tasks

gbbr added a commit to gbbr/opentelemetry-collector-contrib that referenced this issue Jan 3, 2023

extension/{basicauth,bearertoken,oauth2client}: use configopaque.String

20a63ce

Updates open-telemetry#17273

mackjmr mentioned this issue Jul 10, 2023

[receiver/riak] Change the type of Config.Password to be configopaque.String #24062

Merged

mackjmr mentioned this issue Jul 10, 2023

[receiver/saphana] Change the type of Config.Password to be configopaque.String #24063

Merged

mackjmr mentioned this issue Jul 10, 2023

[receiver/snmp] Change the types of the Config.{AuthPassword,PrivacyPassword} fields to be of configopaque.String #24064

Merged

mackjmr mentioned this issue Jul 10, 2023

[receiver/snowflake] Change the type of Config.Password to be configopaque.String #24065

Merged

mackjmr mentioned this issue Jul 10, 2023

[receiver/solace] Change the type of Config.Auth.PlainText.Password to be configopaque.String #24066

Merged

mackjmr mentioned this issue Jul 10, 2023

[receiver/vcenter] Change the type of Config.Password to be configopaque.String #24067

Merged

mx-psi mentioned this issue Jul 10, 2023

[chloggen] Add new change_type value for library API changes open-telemetry/opentelemetry-go-build-tools#360

Closed

jpkrohling pushed a commit that referenced this issue Jul 11, 2023

[receiver/cloudfoundry] Change the type of Config.UAA.Password to b…

71983a4

…e `configopaque.String` (#23832) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

jpkrohling pushed a commit that referenced this issue Jul 12, 2023

[receiver/saphana] Change the type of Config.Password to be `config…

7be2b3a

…opaque.String` (#24063) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

codeboten pushed a commit that referenced this issue Jul 18, 2023

[receiver/snmp] Change the types of the `Config.{AuthPassword,Privacy…

f7fbcd2

…Password}` fields to be of `configopaque.String` (#24064) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

codeboten pushed a commit that referenced this issue Jul 18, 2023

[receiver/snowflake] Change the type of Config.Password to be `conf…

4232e5e

…igopaque.String` (#24065) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

codeboten pushed a commit that referenced this issue Jul 18, 2023

[receiver/vcenter] Change the type of Config.Password to be `config…

24e9bec

…opaque.String` (#24067) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

mx-psi pushed a commit that referenced this issue Jul 19, 2023

[receiver/jmx] Change the types of `Config.{Password,KeystorePassword…

d715dab

…,TruststorePassword}` to be `configopaque.String` (#23864) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

dmitryax pushed a commit that referenced this issue Jul 20, 2023

[receiver/riak] Change the type of Config.Password to be `configopa…

91d1cd8

…que.String` (#24062) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

dmitryax pushed a commit that referenced this issue Jul 20, 2023

[receiver/redis] Change the type of Config.Password to be `configop…

ab92b58

…aque.String` (#24060) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273 Co-authored-by: Alex Boten <aboten@lightstep.com>

dmitryax pushed a commit that referenced this issue Jul 20, 2023

[receiver/awsfirehose] Change the type of Config.AccessKey to be `c…

b72fded

…onfigopaque.String` (#23829) **Description:** Split out from: #17353 **Link to tracking Issue:** #17273

mx-psi closed this as completed Jul 21, 2023

Frapschen mentioned this issue Aug 2, 2023

[exporter/clickhouse] Change the type of Config.Password to be configopaque.String #24778

Merged

crobert-1 mentioned this issue Oct 18, 2023

[exporter/cassandra] Added authorization by username and password #27841

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[all][meta] Make existing sensitive configuration fields opaque #17273

[all][meta] Make existing sensitive configuration fields opaque #17273

mx-psi commented Dec 27, 2022 •

edited

Loading

This comment was marked as resolved.

mx-psi commented Dec 28, 2022

dyladan commented Dec 28, 2022

atoulme commented Dec 28, 2022

mx-psi commented Dec 29, 2022

dyladan commented Dec 30, 2022

gbbr commented Jan 2, 2023 •

edited

Loading

gbbr commented Jan 2, 2023 •

edited

Loading

gbbr commented Jan 2, 2023 •

edited

Loading

gbbr commented Jan 3, 2023

gbbr commented Jan 3, 2023

MovieStoreGuy commented Jan 9, 2023

mx-psi commented Jan 10, 2023

mackjmr commented Jul 20, 2023

mx-psi commented Jul 21, 2023

[all][meta] Make existing sensitive configuration fields opaque #17273

[all][meta] Make existing sensitive configuration fields opaque #17273

Comments

mx-psi commented Dec 27, 2022 • edited Loading

Overview

How to make the change

List of subtasks

Receivers

Processors

Exporters

Extensions

This comment was marked as resolved.

mx-psi commented Dec 28, 2022

dyladan commented Dec 28, 2022

atoulme commented Dec 28, 2022

mx-psi commented Dec 29, 2022

dyladan commented Dec 30, 2022

gbbr commented Jan 2, 2023 • edited Loading

gbbr commented Jan 2, 2023 • edited Loading

gbbr commented Jan 2, 2023 • edited Loading

gbbr commented Jan 3, 2023

gbbr commented Jan 3, 2023

MovieStoreGuy commented Jan 9, 2023

mx-psi commented Jan 10, 2023

mackjmr commented Jul 20, 2023

mx-psi commented Jul 21, 2023

mx-psi commented Dec 27, 2022 •

edited

Loading

gbbr commented Jan 2, 2023 •

edited

Loading

gbbr commented Jan 2, 2023 •

edited

Loading

gbbr commented Jan 2, 2023 •

edited

Loading