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

dynatraceexporter: Add warning to user for insecure storage of user credentials at rest #2231

Closed
alolita opened this issue Jan 30, 2021 · 1 comment
Closed

dynatraceexporter: Add warning to user for insecure storage of user credentials at rest #2231

alolita opened this issue Jan 30, 2021 · 1 comment
Assignees
@flands

Comments

@alolita
Copy link
Member

alolita commented Jan 30, 2021

Is your feature request related to a problem? Please describe.
When using the the dynatraceexporter to connect with the vendor backend service the user has to input API tokens in plain text which is exposed at rest. This is a security exposure that needs to be communicated to the user.

Describe the solution you'd like
The solution proposed includes -

  • adding a warning which is actively communicated to the user
  • recommend storing the token securely at rest (encryption at rest)
  • recommend adding clear documentation itemizing security risks bundled in the exporter folder

Additional context
Unit tests exist for checking unauthorized access but these tests are not enough for an user to understand this security risk. See related unit test -
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/dynatraceexporter/metrics_exporter_test.go#L29
@mx-psi mx-psi mentioned this issue Feb 10, 2021
Closed
@arminru
Copy link
Member

arminru commented Feb 11, 2021

Thanks for bringing this up @alolita.
Following the discussion on #2232, I agree that we should aim for a common solution to make users aware of the risks and provide guidance. I suggest adding a link to the security document being added in open-telemetry/opentelemetry-collector#2461 to exporter/dynatraceexporter/README.md (as well as the other exporters) once it is merged to make it more visible to users.

@gramidt gramidt mentioned this issue Feb 12, 2021
Closed
@flands flands mentioned this issue Feb 13, 2021
Merged
@flands flands self-assigned this Feb 13, 2021
flands added a commit to flands/opentelemetry-collector-contrib that referenced this issue Feb 13, 2021
@flands
Add information about security documentation to exporter README
41e45ba 
Attempts to address:

- open-telemetry#2230
- open-telemetry#2231
- open-telemetry#2232
- open-telemetry#2233
@flands flands mentioned this issue Feb 13, 2021
Merged
bogdandrutu pushed a commit that referenced this issue Feb 16, 2021
@flands
Add information about security documentation to exporter README (#2341)
e7415d7 
Attempts to address:

- #2230
- #2231
- #2232
- #2233
@flands flands closed this as completed Feb 17, 2021
kisieland referenced this issue in kisieland/opentelemetry-collector-contrib Mar 16, 2021
@KSerrania
[config] Default config env variable expansion (#2231)
d8e65bd 
Adds the ability for components to use environment variables in string fields in the default configuration returned by `CreateDefaultConfig`. These fields get expanded when the component is loaded, in the same way the fields of configuration yaml files are (`$FOO` is replaced by the value of the `FOO` environment variable, `$$FOO` is replaced by `$FOO`, `$$$FOO` is replaced by `$`followed by the contents of `FOO`).

For instance, if `CreateDefaultConfig` of a component returns:
```
&Config{
  ...
  TagsConfig: &TagsConfig{
    Env: "$DD_ENV",
  }
  ...
}
```

and the `DD_ENV` environment variable is set to `prod`, then the resulting struct will contain:
```
&Config{
  ...
  TagsConfig: &TagsConfig{
    Env: "prod",
  }
  ...
}
```

**Note:** The default config is expanded _before_ it's merged with the user-provided config, so as to not mess with the latter.

**Link to tracking Issue:** n/a

**Testing:** 

Added unit tests to check that the variable expansion works, and that it doesn't crash in edge cases (unexported private fields that can't be modified, uninitialised config object).

Tested behavior with the Datadog exporter.
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this issue Apr 28, 2021
@flands @pmatyjasek-sumo
Add information about security documentation to exporter README (#2341)
e1d6639 
Attempts to address:

- open-telemetry#2230
- open-telemetry#2231
- open-telemetry#2232
- open-telemetry#2233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@flands flands

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@flands @alolita @arminru