Created
June 18, 2024 11:31
-
-
Save hagen1778/bc7bf45ccab501a7e80fb554e91d9ea1 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md | |
index 22284b655..34aa04f85 100644 | |
--- a/docs/CHANGELOG.md | |
+++ b/docs/CHANGELOG.md | |
@@ -32,7 +32,7 @@ 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.** | |
-* 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. | |
+* 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. | |
* FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `idleConnTimeout` flag set to 50s by default. It should reduce the probability of `broken pipe` or `connection reset by peer` errors in vmauth logs. | |
diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go | |
index 7814eab20..40573003a 100644 | |
--- a/lib/httpserver/httpserver.go | |
+++ b/lib/httpserver/httpserver.go | |
@@ -443,7 +443,7 @@ func CheckAuthFlag(w http.ResponseWriter, r *http.Request, flagValue string, fla | |
if flagValue == "" { | |
return CheckBasicAuth(w, r) | |
} | |
- if !ConstantTimeEqual([]byte(r.FormValue("authKey")), []byte(flagValue)) { | |
+ if !constantTimeEqual(r.FormValue("authKey"), flagValue) { | |
authKeyRequestErrors.Inc() | |
http.Error(w, fmt.Sprintf("The provided authKey doesn't match -%s", flagName), http.StatusUnauthorized) | |
return false | |
@@ -460,7 +460,7 @@ func CheckBasicAuth(w http.ResponseWriter, r *http.Request) bool { | |
} | |
username, password, ok := r.BasicAuth() | |
if ok { | |
- if ConstantTimeEqual([]byte(username), []byte(*httpAuthUsername)) && ConstantTimeEqual([]byte(password), []byte(httpAuthPassword.Get())) { | |
+ if constantTimeEqual(username, *httpAuthUsername) && constantTimeEqual(password, httpAuthPassword.Get()) { | |
return true | |
} | |
authBasicRequestErrors.Inc() | |
@@ -714,11 +714,15 @@ func LogError(req *http.Request, errStr string) { | |
logger.Errorf("uri: %s, remote address: %q: %s", uri, remoteAddr, errStr) | |
} | |
-// ConstantTimeEqual compares two slices of byte in constant-time. | |
+// constantTimeEqual compares two strings in constant-time. | |
// | |
// It returns true if they are equal, else it returns false. | |
-func ConstantTimeEqual(a, b []byte) bool { | |
- eq := subtle.ConstantTimeEq(int32(len(a)), int32(len(b))) | |
- eq &= subtle.ConstantTimeCompare(a, b) | |
- return eq == 1 | |
+func constantTimeEqual(s1, s2 string) bool { | |
+ a := []byte(s1) | |
+ b := []byte(s2) | |
+ // check length explicitly because ConstantTimeCompare doesn't spend time on comparing length | |
+ if subtle.ConstantTimeEq(int32(len(a)), int32(len(b))) == 0 { | |
+ return false | |
+ } | |
+ return subtle.ConstantTimeCompare(a, b) == 1 | |
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment