rms/VictoriaMetrics
1
0
Fork 0
mirror of https://github.com/VictoriaMetrics/VictoriaMetrics.git synced 2026-05-21 18:56:31 +03:00
Code Issues Packages Projects Releases Wiki Activity

lib/httpserver: revert 9b7e532172

Browse Source 
Reason for revert: this commit doesn't resolve real security issues,
while it complicates the resulting code in subtle ways (aka security circus).

Comparison of two strings (passwords, auth keys) takes a few nanoseconds.
This comparison is performed in non-trivial http handler, which takes thousands
of nanoseconds, and the request handler timing is non-deterministic because of Go runtime,
Go GC and other concurrently executed goroutines. The request handler timing is even
more non-deterministic when the application is executed in shared environments
such as Kubernetes, where many other applications may run on the same host and use
shared resources of this host (CPU, RAM bandwidth, network bandwidth).

Additionally, it is expected that the passwords and auth keys are passed via TLS-encrypted connections.
Establishing TLS connections takes additional non-trivial time (millions of nanoseconds),
which depends on many factors such as network latency, network congestion, etc.

This makes impossible to conduct timing attack on passwords and auth keys in VictoriaMetrics components.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6423/files
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6392
This commit is contained in:
Aliaksandr Valialkin
2024-06-25 01:34:46 +02:00
parent de7450b7e0
commit 82d639411d
2 changed files with 2 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
lib/httpserver/httpserver.go
View File

@@ -2,7 +2,6 @@ package httpserver
import (
"context"
"crypto/subtle"
"crypto/tls"
_ "embed"
"errors"
@@ -443,7 +442,7 @@ func CheckAuthFlag(w http.ResponseWriter, r *http.Request, flagValue string, fla
if flagValue == "" {
return CheckBasicAuth(w, r)
}
if !constantTimeEqual(r.FormValue("authKey"), flagValue) {
if 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 +459,7 @@ func CheckBasicAuth(w http.ResponseWriter, r *http.Request) bool {
}
username, password, ok := r.BasicAuth()
if ok {
if constantTimeEqual(username, *httpAuthUsername) && constantTimeEqual(password, httpAuthPassword.Get()) {
if username == *httpAuthUsername && password == httpAuthPassword.Get() {
return true
}
authBasicRequestErrors.Inc()
@@ -713,16 +712,3 @@ func LogError(req *http.Request, errStr string) {
remoteAddr := GetQuotedRemoteAddr(req)
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
}