diff --git a/app/vmbackup/main.go b/app/vmbackup/main.go index 3603f2105..9f18cb41b 100644 --- a/app/vmbackup/main.go +++ b/app/vmbackup/main.go @@ -3,7 +3,6 @@ package main import ( "flag" "fmt" - "net/url" "os" "path/filepath" "strings" @@ -27,9 +26,9 @@ var ( httpListenAddr = flag.String("httpListenAddr", ":8420", "TCP address for exporting metrics at /metrics page") storageDataPath = flag.String("storageDataPath", "victoria-metrics-data", "Path to VictoriaMetrics data. Must match -storageDataPath from VictoriaMetrics or vmstorage") snapshotName = flag.String("snapshotName", "", "Name for the snapshot to backup. See https://docs.victoriametrics.com/single-server-victoriametrics/#how-to-work-with-snapshots. There is no need in setting -snapshotName if -snapshot.createURL is set") - snapshotCreateURL = flag.String("snapshot.createURL", "", "VictoriaMetrics create snapshot url. When this is given a snapshot will automatically be created during backup. "+ + snapshotCreateURL = flagutil.NewURL("snapshot.createURL", "VictoriaMetrics create snapshot url. When this is given a snapshot will automatically be created during backup. "+ "Example: http://victoriametrics:8428/snapshot/create . There is no need in setting -snapshotName if -snapshot.createURL is set") - snapshotDeleteURL = flag.String("snapshot.deleteURL", "", "VictoriaMetrics delete snapshot url. Optional. Will be generated from -snapshot.createURL if not provided. "+ + snapshotDeleteURL = flagutil.NewURL("snapshot.deleteURL", "VictoriaMetrics delete snapshot url. Optional. Will be generated from -snapshot.createURL if not provided. "+ "All created snapshots will be automatically deleted. Example: http://victoriametrics:8428/snapshot/delete") dst = flag.String("dst", "", "Where to put the backup on the remote storage. "+ "Example: gs://bucket/path/to/backup, s3://bucket/path/to/backup, azblob://container/path/to/backup or fs:///path/to/local/backup/dir\n"+ @@ -55,31 +54,23 @@ func main() { // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2055 deleteSnapshot := func() {} - if len(*snapshotCreateURL) > 0 { - // create net/url object - createURL, err := url.Parse(*snapshotCreateURL) - if err != nil { - logger.Fatalf("cannot parse snapshotCreateURL: %s", err) - } + if len(snapshotCreateURL.String()) > 0 { if len(*snapshotName) > 0 { logger.Fatalf("-snapshotName shouldn't be set if -snapshot.createURL is set, since snapshots are created automatically in this case") } - logger.Infof("Snapshot create url %s", createURL.Redacted()) - if len(*snapshotDeleteURL) <= 0 { - err := flag.Set("snapshot.deleteURL", strings.Replace(*snapshotCreateURL, "/create", "/delete", 1)) + + logger.Infof("Snapshot create url %s", snapshotCreateURL.String()) + if len(snapshotDeleteURL.String()) <= 0 { + err := flag.Set("snapshot.deleteURL", strings.Replace(snapshotCreateURL.Get(), "/create", "/delete", 1)) if err != nil { logger.Fatalf("Failed to set snapshot.deleteURL flag: %v", err) } } - deleteURL, err := url.Parse(*snapshotDeleteURL) - if err != nil { - logger.Fatalf("cannot parse snapshotDeleteURL: %s", err) - } - logger.Infof("Snapshot delete url %s", deleteURL.Redacted()) - name, err := snapshot.Create(createURL.String()) + logger.Infof("Snapshot delete url %s", snapshotDeleteURL) + name, err := snapshot.Create(snapshotCreateURL.Get()) if err != nil { - logger.Fatalf("cannot create snapshot: %s", err) + logger.Fatalf("cannot create snapshot via %q: %s", snapshotCreateURL, err) } err = flag.Set("snapshotName", name) if err != nil { @@ -87,9 +78,9 @@ func main() { } deleteSnapshot = func() { - err := snapshot.Delete(deleteURL.String(), name) + err := snapshot.Delete(snapshotDeleteURL.Get(), name) if err != nil { - logger.Fatalf("cannot delete snapshot: %s", err) + logger.Fatalf("cannot delete snapshot via %q: %s", snapshotDeleteURL, err) } } } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 99f4b4b0c..6450cdd4e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -41,6 +41,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): support regex matching when routing incoming requests based on HTTP [query args](https://en.wikipedia.org/wiki/Query_string) via `src_query_args` option at `url_map`. See [these docs](https://docs.victoriametrics.com/vmauth/#generic-http-proxy-for-different-backends) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6070). * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): optimize auto-suggestion performance for metric names when the database contains big number of unique time series. * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): in the Select component, user-entered values are now preserved on blur if they match options in the list. +* FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup): support loading values for flags `-snapshot.createURL` and `-snapshot.deleteURL` from files for security reasons. To load value from file use the following format `-snapshot.createURL=file:///abs/path/to/file` or `-snapshot.deleteURL=file://./relative/path/to/file`. See related [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5973). +* FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup): hide sensitive information for flags `-snapshot.createURL` and `-snapshot.deleteURL` when printing logs. The change masks HTTP basic authentication user and password values, as well as GET params matching `auth`, `pass`, `key`, `secret`, `token` words. See related [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5973). Thanks to @wasim-nihal for [initial implementation](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6060). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): supported any status codes from the range 200-299 from alertmanager. Previously, only 200 status code considered a successful action. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6110). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): avoid blocking `/api/v1/rules`, `/api/v1/alerts`, `/metrics` APIs when alerting rule uses template functions `query`, which could takes a while to execute. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6079). diff --git a/lib/flagutil/url.go b/lib/flagutil/url.go new file mode 100644 index 000000000..f8dcf8f65 --- /dev/null +++ b/lib/flagutil/url.go @@ -0,0 +1,159 @@ +package flagutil + +import ( + "flag" + "fmt" + "log" + "net/url" + "os" + "regexp" + "strings" + "sync/atomic" + "unicode" + + "github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime" +) + +// NewURL returns new `url` flag with the given name and description. +// +// The url value is redacted when calling URL.String() in the following way: +// 1. Basic Auth username and password are replaced with "xxxxx" +// 2. Values of GET params matching `secretWordsRe` expression are replaced with "xxxxx". +// +// Call URL.Get() for obtaining original URL address. +func NewURL(name, description string) *URL { + description += fmt.Sprintf("\nFlag value can be read from the given file when using -%s=file:///abs/path/to/file or -%s=file://./relative/path/to/file . ", name, name) + u := &URL{ + flagname: name, + } + ru := &redactedURL{ + URL: &url.URL{}, + redacted: "", + } + u.value.Store(&ru) + flag.Var(u, name, description) + return u +} + +// URL is a flag holding URL address +// +// If the flag value is file:///path/to/file, +// then its contents is automatically re-read from the given file on disk. +type URL struct { + nextRefreshTimestamp atomic.Uint64 + + value atomic.Pointer[*redactedURL] + + // flagname is the name of the flag + flagname string + + // sourcePath contains either url or path to file with the url + sourcePath string +} + +type redactedURL struct { + *url.URL + redacted string +} + +// Get returns the current u address. +// +// It re-reads u value from the file:///path/to/file +// if they were passed to URL.Set. +func (u *URL) Get() string { + u.maybeRereadURL() + ru := *u.value.Load() + return ru.URL.String() +} + +// Get returns the current u redacted address. +// +// It re-reads u value from the file:///path/to/file +// if they were passed to URL.Set. +func (u *URL) String() string { + u.maybeRereadURL() + ru := *u.value.Load() + return ru.redacted +} + +func (u *URL) maybeRereadURL() { + if u.sourcePath == "" { + // Fast path - nothing to re-read + return + } + tsCurr := fasttime.UnixTimestamp() + tsNext := u.nextRefreshTimestamp.Load() + if tsCurr < tsNext { + // Fast path - nothing to re-read + return + } + + // Re-read value from s.sourcePath + u.nextRefreshTimestamp.Store(tsCurr + 2) + data, err := os.ReadFile(u.sourcePath) + if err != nil { + // cannot use lib/logger, since it can be uninitialized yet + log.Printf("flagutil: fall back to the previous url for -%s, since failed to re-read it from %q: cannot read %q: %s\n", u.flagname, u.sourcePath, u.sourcePath, err.Error()) + } else { + addr := strings.TrimRightFunc(string(data), unicode.IsSpace) + res, err := newRedactedURL(addr) + if err != nil { + log.Printf("flagutil: cannot parse %q: %s\n", u.flagname, err.Error()) + return + } + u.value.Store(&res) + } +} + +// Set implements flag.Value interface. +func (u *URL) Set(value string) error { + u.nextRefreshTimestamp.Store(0) + var s string + switch { + case strings.HasPrefix(value, "file://"): + u.sourcePath = strings.TrimPrefix(value, "file://") + data, err := os.ReadFile(u.sourcePath) + if err != nil { + // cannot use lib/logger, since it can be uninitialized yet + return fmt.Errorf("cannot read %q: %w", u.sourcePath, err) + } + s = strings.TrimRightFunc(string(data), unicode.IsSpace) + default: + u.sourcePath = "" + s = value + } + + res, err := newRedactedURL(s) + if err != nil { + return fmt.Errorf("cannot parse %q: %s", u.flagname, err) + } + u.value.Store(&res) + return nil +} + +var secretWordsRe = regexp.MustCompile("auth|pass|key|secret|token") + +func newRedactedURL(s string) (*redactedURL, error) { + u, err := url.Parse(s) + if err != nil { + return nil, fmt.Errorf("cannot parse URL: %s", err) + } + ru := &redactedURL{URL: u} + + // copy URL before mutating query params + u2 := *u + values := u2.Query() + for k, vs := range values { + if secretWordsRe.MatchString(k) { + for i := range vs { + vs[i] = "xxxxx" + } + } + } + u2.RawQuery = values.Encode() + if _, has := u2.User.Password(); has { + u2.User = url.UserPassword("xxxxx", "xxxxx") + } + ru.redacted = u2.String() + return ru, nil +} diff --git a/lib/flagutil/url_test.go b/lib/flagutil/url_test.go new file mode 100644 index 000000000..4b36f5486 --- /dev/null +++ b/lib/flagutil/url_test.go @@ -0,0 +1,45 @@ +package flagutil + +import ( + "os" + "testing" +) + +func TestNewURL(t *testing.T) { + u := &URL{} + f := func(s, exp string) { + t.Helper() + if err := u.Set(s); err != nil { + t.Fatalf("failed to set %q value: %s", s, err) + } + if u.String() != exp { + t.Fatalf("expected to get %q; got %q instead", exp, u.String()) + } + } + + f("", "") + f("http://foo:8428", "http://foo:8428") + f("http://username:password@foo:8428", "http://xxxxx:xxxxx@foo:8428") + f("http://foo:8428?authToken=bar", "http://foo:8428?authToken=xxxxx") + f("http://username:password@foo:8428?authToken=bar", "http://xxxxx:xxxxx@foo:8428?authToken=xxxxx") + + file, err := os.CreateTemp("", "") + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.Remove(file.Name()) }() + + writeToFile(t, file.Name(), "http://foo:8428") + f("file://"+file.Name(), "http://foo:8428") + + writeToFile(t, file.Name(), "http://xxxxx:password@foo:8428?authToken=bar") + f("file://"+file.Name(), "http://xxxxx:xxxxx@foo:8428?authToken=xxxxx") +} + +func writeToFile(t *testing.T, file, b string) { + t.Helper() + err := os.WriteFile(file, []byte(b), 0644) + if err != nil { + t.Fatal(err) + } +} diff --git a/lib/snapshot/snapshot.go b/lib/snapshot/snapshot.go index 87bf8d09f..c4324ea81 100644 --- a/lib/snapshot/snapshot.go +++ b/lib/snapshot/snapshot.go @@ -51,13 +51,13 @@ func Create(createSnapshotURL string) (string, error) { return "", err } if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("unexpected status code returned from %q: %d; expecting %d; response body: %q", u.Redacted(), resp.StatusCode, http.StatusOK, body) + return "", fmt.Errorf("unexpected status code: %d; expecting %d; response body: %q", resp.StatusCode, http.StatusOK, body) } snap := snapshot{} err = json.Unmarshal(body, &snap) if err != nil { - return "", fmt.Errorf("cannot parse JSON response from %q: %w; response body: %q", u.Redacted(), err, body) + return "", fmt.Errorf("cannot parse JSON response: %w; response body: %q", err, body) } if snap.Status == "ok" { @@ -67,7 +67,7 @@ func Create(createSnapshotURL string) (string, error) { if snap.Status == "error" { return "", errors.New(snap.Msg) } - return "", fmt.Errorf("Unkown status: %v", snap.Status) + return "", fmt.Errorf("unknown status: %v", snap.Status) } // Delete deletes a snapshot via the provided api endpoint @@ -95,13 +95,13 @@ func Delete(deleteSnapshotURL string, snapshotName string) error { return err } if resp.StatusCode != http.StatusOK { - return fmt.Errorf("unexpected status code returned from %q: %d; expecting %d; response body: %q", u.Redacted(), resp.StatusCode, http.StatusOK, body) + return fmt.Errorf("unexpected status code: %d; expecting %d; response body: %q", resp.StatusCode, http.StatusOK, body) } snap := snapshot{} err = json.Unmarshal(body, &snap) if err != nil { - return fmt.Errorf("cannot parse JSON response from %q: %w; response body: %q", u.Redacted(), err, body) + return fmt.Errorf("cannot parse JSON response: %w; response body: %q", err, body) } if snap.Status == "ok" { @@ -111,5 +111,5 @@ func Delete(deleteSnapshotURL string, snapshotName string) error { if snap.Status == "error" { return errors.New(snap.Msg) } - return fmt.Errorf("Unkown status: %v", snap.Status) + return fmt.Errorf("unknown status: %v", snap.Status) }