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

Scraping Service: basic_auth gets wiped by marshaling to YAML #62

Closed
rfratto opened this issue May 8, 2020 · 1 comment · Fixed by #63 or #67
Closed

Scraping Service: basic_auth gets wiped by marshaling to YAML #62

rfratto opened this issue May 8, 2020 · 1 comment · Fixed by #63 or #67
Labels
bug Something isn't working frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.

Comments

@rfratto
Copy link
Member

rfratto commented May 8, 2020 •
edited
Loading

Prometheus stores the basic_auth as a Secret type which implements yaml.Marshaler and always forces the string to be <secret>. This breaks the configuration storage.

The yamlCodec in pkg/prometheus/ha should wrap the instance.Config type and store secret values separately. The following are secrets:

  • BasicAuth Password
  • HTTP Client bearer token

Note that for the same reason, hashing configs when detecting if configs changed is broken; the value being hashed is not the secret but rather the string <secret>. To fix this, the secret values above should be added separately to the hashing function if they are non-nil and non-empty.
@rfratto rfratto added the bug Something isn't working label May 8, 2020
rfratto referenced this issue in rfratto/agent May 8, 2020
@rfratto
pkg/prometheus/ha: store and hash secrets properly
7a827ac 
The Bearer Token and Basic Auth Password for remote_write configs were
not being stored or hashed properly; those values are of a Secret type
that implements a custom yaml.Marshaler and removes the underlying
secret value.

So when configs were being stored in the KV store, the secrets were
being removed and replaced with the text "<secret>". This caused
agents who loaded the config to run an instance to use the wrong
password.

Hashing a config had the same problem; since the hash only marshaled a
config to YAML, secrets were not being calculated properly with the
hash.

This commit fixes both problems; secrets are stored separately now in a
raw string type and hashing adds the values for secrets before summing
the input.

Fixes #62.
@rfratto rfratto mentioned this issue May 8, 2020
Merged
rfratto referenced this issue in rfratto/agent May 8, 2020
@rfratto
pkg/prometheus/ha: store and hash secrets properly
05405bc 
The Bearer Token and Basic Auth Password for remote_write configs were
not being stored or hashed properly; those values are of a Secret type
that implements a custom yaml.Marshaler and removes the underlying
secret value.

So when configs were being stored in the KV store, the secrets were
being removed and replaced with the text "<secret>". This caused
agents who loaded the config to run an instance to use the wrong
password.

Hashing a config had the same problem; since the hash only marshaled a
config to YAML, secrets were not being calculated properly with the
hash.

This commit fixes both problems; secrets are stored separately now in a
raw string type and hashing adds the values for secrets before summing
the input.

Fixes #62.
rfratto added a commit that referenced this issue May 11, 2020
@rfratto
pkg/prometheus/ha: store and hash secrets properly (#63)
3dd3fe8 
The Bearer Token and Basic Auth Password for remote_write configs were
not being stored or hashed properly; those values are of a Secret type
that implements a custom yaml.Marshaler and removes the underlying
secret value.

So when configs were being stored in the KV store, the secrets were
being removed and replaced with the text "<secret>". This caused
agents who loaded the config to run an instance to use the wrong
password.

Hashing a config had the same problem; since the hash only marshaled a
config to YAML, secrets were not being calculated properly with the
hash.

This commit fixes both problems; secrets are stored separately now in a
raw string type and hashing adds the values for secrets before summing
the input.

Fixes #62.
@rfratto
Copy link
Member Author

rfratto commented May 11, 2020

This isn't fixed. The YAML is being remarshaled all over the place, including from agentctl. This needs a more general solution.

@rfratto rfratto reopened this May 11, 2020
rfratto referenced this issue in rfratto/agent May 11, 2020
@rfratto
Duplicate RemoteWriteConfig without the Secret type
7c86420 
This commit completely removes the Secret type to allow
RemoteWriteConfigs to be arbitrarily marshalled back and forth between
YAML. Sanitization now happens on demand rather than being forced by the
marshaling.

Closes #62
@rfratto rfratto mentioned this issue May 11, 2020
Merged
rfratto added a commit that referenced this issue May 11, 2020
@rfratto
Duplicate RemoteWriteConfig without the Secret type (#67)
2bdb842 
* Duplicate RemoteWriteConfig without the Secret type

This commit completely removes the Secret type to allow
RemoteWriteConfigs to be arbitrarily marshalled back and forth between
YAML. Sanitization now happens on demand rather than being forced by the
marshaling.

Closes #62

* fix lint error
mattdurham pushed a commit that referenced this issue Nov 11, 2021
@rfratto
pkg/prometheus/ha: store and hash secrets properly (#63)
01aa930 
The Bearer Token and Basic Auth Password for remote_write configs were
not being stored or hashed properly; those values are of a Secret type
that implements a custom yaml.Marshaler and removes the underlying
secret value.

So when configs were being stored in the KV store, the secrets were
being removed and replaced with the text "<secret>". This caused
agents who loaded the config to run an instance to use the wrong
password.

Hashing a config had the same problem; since the hash only marshaled a
config to YAML, secrets were not being calculated properly with the
hash.

This commit fixes both problems; secrets are stored separately now in a
raw string type and hashing adds the values for secrets before summing
the input.

Fixes #62.
mattdurham pushed a commit that referenced this issue Nov 11, 2021
@rfratto
Duplicate RemoteWriteConfig without the Secret type (#67)
b0525e4 
* Duplicate RemoteWriteConfig without the Secret type

This commit completely removes the Secret type to allow
RemoteWriteConfigs to be arbitrarily marshalled back and forth between
YAML. Sanitization now happens on demand rather than being forced by the
marshaling.

Closes #62

* fix lint error
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 26, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Milestone
No milestone
1 participant
@rfratto