mirror of
https://github.com/VictoriaMetrics/VictoriaMetrics.git
synced 2024-11-21 14:44:00 +00:00
victoria-metrics: constant-time comparison of credentials like authkeys and basic auth credentials (#6423)
Changes for constant-time comparison of credentials like authkeys and basic auth credentials. See: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6392 --------- Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
This commit is contained in:
parent
13e3bb88a9
commit
9b7e532172
2 changed files with 17 additions and 2 deletions
|
@ -32,6 +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.**
|
**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. 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: [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: [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.
|
* 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.
|
||||||
|
|
|
@ -2,6 +2,7 @@ package httpserver
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"crypto/subtle"
|
||||||
"crypto/tls"
|
"crypto/tls"
|
||||||
_ "embed"
|
_ "embed"
|
||||||
"errors"
|
"errors"
|
||||||
|
@ -442,7 +443,7 @@ func CheckAuthFlag(w http.ResponseWriter, r *http.Request, flagValue string, fla
|
||||||
if flagValue == "" {
|
if flagValue == "" {
|
||||||
return CheckBasicAuth(w, r)
|
return CheckBasicAuth(w, r)
|
||||||
}
|
}
|
||||||
if r.FormValue("authKey") != flagValue {
|
if !constantTimeEqual(r.FormValue("authKey"), flagValue) {
|
||||||
authKeyRequestErrors.Inc()
|
authKeyRequestErrors.Inc()
|
||||||
http.Error(w, fmt.Sprintf("The provided authKey doesn't match -%s", flagName), http.StatusUnauthorized)
|
http.Error(w, fmt.Sprintf("The provided authKey doesn't match -%s", flagName), http.StatusUnauthorized)
|
||||||
return false
|
return false
|
||||||
|
@ -459,7 +460,7 @@ func CheckBasicAuth(w http.ResponseWriter, r *http.Request) bool {
|
||||||
}
|
}
|
||||||
username, password, ok := r.BasicAuth()
|
username, password, ok := r.BasicAuth()
|
||||||
if ok {
|
if ok {
|
||||||
if username == *httpAuthUsername && password == httpAuthPassword.Get() {
|
if constantTimeEqual(username, *httpAuthUsername) && constantTimeEqual(password, httpAuthPassword.Get()) {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
authBasicRequestErrors.Inc()
|
authBasicRequestErrors.Inc()
|
||||||
|
@ -712,3 +713,16 @@ func LogError(req *http.Request, errStr string) {
|
||||||
remoteAddr := GetQuotedRemoteAddr(req)
|
remoteAddr := GetQuotedRemoteAddr(req)
|
||||||
logger.Errorf("uri: %s, remote address: %q: %s", uri, remoteAddr, errStr)
|
logger.Errorf("uri: %s, remote address: %q: %s", uri, remoteAddr, errStr)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// constantTimeEqual compares two strings in constant-time.
|
||||||
|
//
|
||||||
|
// It returns true if they are equal, else it returns false.
|
||||||
|
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
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue