fix: obfuscate secrets using a constant length string

0xERR0R · Apr 2, 2024 · 1edf8cc · 1edf8cc
1 parent 2c6b704
commit 1edf8cc
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 27 deletions.
diff --git a/config/config.go b/config/config.go
@@ -27,6 +27,8 @@ const (
 	udpPort   = 53
 	tlsPort   = 853
 	httpsPort = 443
+
+	secretObfuscator = "********"
 )
 
 type Configurable interface {

diff --git a/config/query_log.go b/config/query_log.go
@@ -57,14 +57,10 @@ func (c *QueryLog) censoredTarget() string {
 		return c.Target
 	}
 
-	if target.User == nil {
+	pass, ok := target.User.Password()
+	if !ok {
 		return c.Target
 	}
 
-	// Drop the password since special chars like * get URL escaped
-	if pass, hasPass :=target.User.Password(); hasPass {
-		return strings.Replace(target.String(), pass, strings.Repeat("*", len(pass)), 1)
-	}
-
-	return target.String()
+	return strings.ReplaceAll(c.Target, pass, secretObfuscator)
 }
diff --git a/config/query_log_test.go b/config/query_log_test.go
@@ -56,7 +56,7 @@ var _ = Describe("QueryLogConfig", func() {
 			Expect(hook.Messages).Should(ContainElement(ContainSubstring("logRetentionDays:")))
 		})
 
-		DescribeTable("doesn"t print the target password", func(target string) {
+		DescribeTable("secret censoring", func(target string) {
 			cfg.Type = QueryLogTypeMysql
 			cfg.Target = target
 

diff --git a/config/redis.go b/config/redis.go
@@ -1,8 +1,6 @@
 package config
 
 import (
-	"strings"
-
 	"github.com/sirupsen/logrus"
 )
 
@@ -32,7 +30,7 @@ func (c *Redis) LogConfig(logger *logrus.Entry) {
 	}
 
 	logger.Info("username: ", c.Username)
-	logger.Info("password: ", obfuscatePassword(c.Password))
+	logger.Info("password: ", secretObfuscator)
 	logger.Info("database: ", c.Database)
 	logger.Info("required: ", c.Required)
 	logger.Info("connectionAttempts: ", c.ConnectionAttempts)
@@ -42,16 +40,11 @@ func (c *Redis) LogConfig(logger *logrus.Entry) {
 		logger.Info("sentinel:")
 		logger.Info("  master: ", c.Address)
 		logger.Info("  username: ", c.SentinelUsername)
-		logger.Info("  password: ", obfuscatePassword(c.SentinelPassword))
+		logger.Info("  password: ", secretObfuscator)
 		logger.Info("  addresses:")
 
 		for _, addr := range c.SentinelAddresses {
 			logger.Info("    - ", addr)
 		}
 	}
 }
-
-// obfuscatePassword replaces all characters of a password except the first and last with *
-func obfuscatePassword(pass string) string {
-	return strings.Repeat("*", len(pass))
-}
diff --git a/config/redis_test.go b/config/redis_test.go
@@ -86,19 +86,23 @@ var _ = Describe("Redis", func() {
 						ContainElement(ContainSubstring("  - localhost:26380"))))
 			})
 		})
-	})
 
-	Describe("obfuscatePassword", func() {
-		When("password is empty", func() {
-			It("should return empty string", func() {
-				Expect(obfuscatePassword("")).Should(Equal(""))
-			})
+		const secretValue = "secret-value"
+
+		It("should not log the password", func() {
+			c.Password = secretValue
+			c.LogConfig(logger)
+
+			Expect(hook.Calls).ShouldNot(BeEmpty())
+			Expect(hook.Messages).ShouldNot(ContainElement(ContainSubstring(secretValue)))
 		})
 
-		When("password is not empty", func() {
-			It("should return obfuscated password", func() {
-				Expect(obfuscatePassword("test123")).Should(Equal("*******"))
-			})
+		It("should not log the sentinel password", func() {
+			c.SentinelPassword = secretValue
+			c.LogConfig(logger)
+
+			Expect(hook.Calls).ShouldNot(BeEmpty())
+			Expect(hook.Messages).ShouldNot(ContainElement(ContainSubstring(secretValue)))
 		})
 	})
 })