lib/promauth: follow-up after 006b8c7534

- Take into account `ca`, `key` and `cert` values when generating string representation of TLSConfig.
  Print hashes instead of real values because of security considerations.
- Properly update Config.tlsCertDigets when `key` and `cert` values are set.
  This allows properly updating scrape targets after these values are updated in configs.
- Do not re-generate certificate from `key` and `cert` values per each call to getTLSCert,
  because these values are immutable.
- Do not set `ca` value from `ca_file` value, so it isn't exposed at `/config` page.
- Generate proper error messages on incorrect `key`, `cert` or `ca` values.

lib/promauth/config.go

@@ -15,6 +15,7 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/fs"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
+	xxhash "github.com/cespare/xxhash/v2"
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/clientcredentials"
 )
@@ -84,8 +85,11 @@ func (tlsConfig *TLSConfig) String() string {
 	if tlsConfig == nil {
 		return ""
 	}
-	return fmt.Sprintf("ca_file=%q, cert_file=%q, key_file=%q, server_name=%q, insecure_skip_verify=%v, min_version=%q",
+	caHash := xxhash.Sum64(tlsConfig.CA)
-		tlsConfig.CAFile, tlsConfig.CertFile, tlsConfig.KeyFile, tlsConfig.ServerName, tlsConfig.InsecureSkipVerify, tlsConfig.MinVersion)
+	certHash := xxhash.Sum64(tlsConfig.Cert)
+	keyHash := xxhash.Sum64(tlsConfig.Key)
+	return fmt.Sprintf("hash(ca)=%d, ca_file=%q, hash(cert)=%d, cert_file=%q, hash(key)=%d, key_file=%q, server_name=%q, insecure_skip_verify=%v, min_version=%q",
+		caHash, tlsConfig.CAFile, certHash, tlsConfig.CertFile, keyHash, tlsConfig.KeyFile, tlsConfig.ServerName, tlsConfig.InsecureSkipVerify, tlsConfig.MinVersion)
 }
 
 // Authorization represents generic authorization config.
@@ -273,6 +277,9 @@ func (ac *Config) GetAuthHeader() string {
 }
 
 // String returns human-readable representation for ac.
+//
+// It is also used for comparing Config objects for equality. If two Config
+// objects have the same string representation, then they are considered equal.
 func (ac *Config) String() string {
 	return fmt.Sprintf("AuthDigest=%s, TLSRootCA=%s, TLSCertificate=%s, TLSServerName=%s, TLSInsecureSkipVerify=%v, TLSMinVersion=%d",
 		ac.authDigest, ac.tlsRootCAString(), ac.tlsCertDigest, ac.TLSServerName, ac.TLSInsecureSkipVerify, ac.TLSMinVersion)
@@ -459,21 +466,24 @@ func NewConfig(baseDir string, az *Authorization, basicAuth *BasicAuthConfig, be
 	if tlsConfig != nil {
 		tlsServerName = tlsConfig.ServerName
 		tlsInsecureSkipVerify = tlsConfig.InsecureSkipVerify
-		if (tlsConfig.CertFile != "" || tlsConfig.KeyFile != "") || (len(tlsConfig.Key) != 0 || len(tlsConfig.Cert) != 0) {
+		if len(tlsConfig.Key) != 0 || len(tlsConfig.Cert) != 0 {
+			cert, err := tls.X509KeyPair(tlsConfig.Cert, tlsConfig.Key)
+			if err != nil {
+				return nil, fmt.Errorf("cannot load TLS certificate from the provided `cert` and `key` values: %w", err)
+			}
+			getTLSCert = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
+				return &cert, nil
+			}
+			h := xxhash.Sum64(tlsConfig.Key) ^ xxhash.Sum64(tlsConfig.Cert)
+			tlsCertDigest = fmt.Sprintf("digest(key+cert)=%d", h)
+		} else if tlsConfig.CertFile != "" || tlsConfig.KeyFile != "" {
 			getTLSCert = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
 				// Re-read TLS certificate from disk. This is needed for https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1420
-				if tlsConfig.CertFile != "" || tlsConfig.KeyFile != "" {
+				certPath := fs.GetFilepath(baseDir, tlsConfig.CertFile)
-					certPath := fs.GetFilepath(baseDir, tlsConfig.CertFile)
+				keyPath := fs.GetFilepath(baseDir, tlsConfig.KeyFile)
-					keyPath := fs.GetFilepath(baseDir, tlsConfig.KeyFile)
+				cert, err := tls.LoadX509KeyPair(certPath, keyPath)
-					cert, err := tls.LoadX509KeyPair(certPath, keyPath)
-					if err != nil {
-						return nil, fmt.Errorf("cannot load TLS certificate from `cert_file`=%q, `key_file`=%q: %w", tlsConfig.CertFile, tlsConfig.KeyFile, err)
-					}
-					return &cert, nil
-				}
-				cert, err := tls.X509KeyPair(tlsConfig.Cert, tlsConfig.Key)
 				if err != nil {
-					return nil, fmt.Errorf("cannot load TLS certificate: %w", err)
+					return nil, fmt.Errorf("cannot load TLS certificate from `cert_file`=%q, `key_file`=%q: %w", tlsConfig.CertFile, tlsConfig.KeyFile, err)
 				}
 				return &cert, nil
 			}
@@ -483,17 +493,19 @@ func NewConfig(baseDir string, az *Authorization, basicAuth *BasicAuthConfig, be
 			}
 			tlsCertDigest = fmt.Sprintf("certFile=%q, keyFile=%q", tlsConfig.CertFile, tlsConfig.KeyFile)
 		}
-		if tlsConfig.CAFile != "" {
+		if len(tlsConfig.CA) != 0 {
+			tlsRootCA = x509.NewCertPool()
+			if !tlsRootCA.AppendCertsFromPEM(tlsConfig.CA) {
+				return nil, fmt.Errorf("cannot parse data from `ca` value")
+			}
+		} else if tlsConfig.CAFile != "" {
 			path := fs.GetFilepath(baseDir, tlsConfig.CAFile)
 			data, err := fs.ReadFileOrHTTP(path)
 			if err != nil {
 				return nil, fmt.Errorf("cannot read `ca_file` %q: %w", tlsConfig.CAFile, err)
 			}
-			tlsConfig.CA = data
-		}
-		if len(tlsConfig.CA) != 0 {
 			tlsRootCA = x509.NewCertPool()
-			if !tlsRootCA.AppendCertsFromPEM(tlsConfig.CA) {
+			if !tlsRootCA.AppendCertsFromPEM(data) {
 				return nil, fmt.Errorf("cannot parse data from `ca_file` %q", tlsConfig.CAFile)
 			}
 		}