mirror of
https://github.com/VictoriaMetrics/VictoriaMetrics.git
synced 2024-11-21 14:44:00 +00:00
lib/fs/fscore: do not trim content from path (#6503)
### Describe Your Changes
Trimming content which is loaded from an external pass leads to obscure
issues in case user-defined input contained trimmed chars. For example.
user-defined password "foo\n" will become "foo" while user will expect
it to contain a new line.
---
For example, a user defines a password which ends with `\n`. This often
happens when user Kubernetes secrets and manually encodes value as
base64-encoded string.
In this case vmauth configuration might look like:
```
users:
- url_prefix:
- http://vminsert:8480/insert/0/prometheus/api/v1/write
name: foo
username: foo
password: "foobar\n"
```
vmagent configuration for this setup will use the following flags:
```
-remoteWrite.url=http://vmauth:8427/
-remoteWrite.basicAuth.passwordFile=/tmp/vmagent-password
-remoteWrite.basicAuth.username="foo"
```
Where `/tmp/vmagent-password` is a file with `foobar\n` password.
Before this change such configuration will result in `401 Unauthorized`
response received by vmagent since after file content will become
`foobar`.
---
An example with Kubernetes operator which uses a secret to reference the
same password in multiple configurations.
<details>
<summary>See full manifests</summary>
`Secret`:
```
apiVersion: v1
data:
name: Zm9v # foo
password: Zm9vYmFy # foobar\n
username: Zm9v= # foo
kind: Secret
metadata:
name: vmuser
```
`VMUser`:
```
apiVersion: operator.victoriametrics.com/v1beta1
kind: VMUser
metadata:
name: vmagents
spec:
generatePassword: false
name: vmagents
targetRefs:
- crd:
kind: VMAgent
name: some-other-agent
namespace: example
username: foo
# note - the secret above is referenced to provide password
passwordRef:
name: vmagent
key: password
```
`VMAgent`:
```
apiVersion: operator.victoriametrics.com/v1beta1
kind: VMAgent
metadata:
name: example
spec:
selectAllByDefault: true
scrapeInterval: 5s
replicaCount: 1
remoteWrite:
- url: "http://vmauth-vmauth-example:8427/api/v1/write"
# note - the secret above is referenced as well
basicAuth:
username:
name: vmagent
key: username
password:
name: vmagent
key: password
```
</details>
Since both config target exactly the same `Secret` object it is expected
to work, but apparently the result will be `401 Unauthrized` error.
### Checklist
The following checks are **mandatory**:
- [x] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).
---------
Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
Signed-off-by: hagen1778 <roman@victoriametrics.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
(cherry picked from commit 201fd6de1e
)
This commit is contained in:
parent
7a21e6cb6b
commit
886f545f81
3 changed files with 6 additions and 6 deletions
|
@ -32,6 +32,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
|
|||
|
||||
**Update note 1: the `--vm-disable-progress-bar` command-line flag at `vmctl` was deprecated. Use `--disable-progress-bar` instead.**
|
||||
|
||||
**Update note 2: `*.passwordFile` and similar flags are no longer trimming trailing whitespaces at the end of content. Make sure to update the templating of password files or HTTP endpoints to not include trailing whitespaces before the upgrade. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6503) PR for the details.**
|
||||
|
||||
* FEATURE: all VictoriaMetrics components: use constant-time comparison for comparing HTTP basic auth credentials and auth keys. This should prevent timing attacks when comparing these credentials. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6392) for details. Thanks to @wasim-nihal for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6423).
|
||||
* FEATURE: [alerts-vmagent](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts-vmagent.yml): add new alerting rules `StreamAggrFlushTimeout` and `StreamAggrDedupFlushTimeout` to notify about issues during stream aggregation.
|
||||
* FEATURE: [dashboards/vmagent](https://grafana.com/grafana/dashboards/12683): add row `Streaming aggregation` with panels related to [streaming aggregation](https://docs.victoriametrics.com/stream-aggregation/) process.
|
||||
|
@ -43,6 +45,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
|
|||
* FEATURE: [vmalert-tool](https://docs.victoriametrics.com/vmalert-tool/): support file path with hierarchical patterns and regexpes, and http url in unittest cmd-line flag `-files`, e.g. `-files="http://<some-server-addr>/path/to/rules"` or `-files="dir/**/*.yaml"`.
|
||||
|
||||
* BUGFIX: all VictoriaMetrics components: prioritize `-configAuthKey` and `-reloadAuthKey` over `-httpAuth.*` settings. This change aligns behavior of mentioned flags with other auth flags like `-metricsAuthKey`, `-flagsAuthKey`, `-pprofAuthKey`. Check [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6329).
|
||||
* BUGFIX: all VictoriaMetrics components: do not trim trailing spaces when reading content from `*.passwordFile` and similar flags. Previously, trailing spaces were trimmed from the content of the password file, which could lead to unexpected authentication errors.
|
||||
* BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl/): add `--disable-progress-bar` global command-line flag. It can be used for disabling dynamic progress bar for all migration modes. `--vm-disable-progress-bar` command-line flag is deprecated and will be removed in the future releases. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6367).
|
||||
* BUGFIX: [stream aggregation](https://docs.victoriametrics.com/stream-aggregation/): prevent [rate_sum](https://docs.victoriametrics.com/stream-aggregation/#rate_sum) and [rate_avg](https://docs.victoriametrics.com/stream-aggregation/#rate_avg) producing `NaN` results for stale time series. Before, when series matched for aggregation became stale or weren't updated during aggregation interval, the `rate_sum` or `rate_avg` could produce data point with `NaN` value. During visualization, such aggregation results would be displayed as gaps in time series.
|
||||
* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): fix path for system links printed on default vmalert's UI page when `-http.pathPrefix` is set.
|
||||
|
|
|
@ -32,7 +32,7 @@ func TestPassword(t *testing.T) {
|
|||
|
||||
// read the password from file by relative path
|
||||
localPassFile := "testdata/password.txt"
|
||||
expectedPassword = "foo-bar-baz"
|
||||
expectedPassword = "foo-bar-baz\n\n\n"
|
||||
path := "file://" + localPassFile
|
||||
if err := p.Set(path); err != nil {
|
||||
t.Fatalf("cannot set password to file: %s", err)
|
||||
|
@ -52,7 +52,7 @@ func TestPassword(t *testing.T) {
|
|||
if err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
}
|
||||
expectedPassword = "foo-bar-baz"
|
||||
expectedPassword = "foo-bar-baz\n\n\n"
|
||||
path = "file://" + localPassFile
|
||||
if err := p.Set(path); err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
|
|
|
@ -7,8 +7,6 @@ import (
|
|||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"unicode"
|
||||
)
|
||||
|
||||
// ReadPasswordFromFileOrHTTP reads password for the give path.
|
||||
|
@ -19,8 +17,7 @@ func ReadPasswordFromFileOrHTTP(path string) (string, error) {
|
|||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
pass := strings.TrimRightFunc(string(data), unicode.IsSpace)
|
||||
return pass, nil
|
||||
return string(data), nil
|
||||
}
|
||||
|
||||
// ReadFileOrHTTP reads path either from local filesystem or from http if path starts with http or https.
|
||||
|
|
Loading…
Reference in a new issue