From 83b405698539c5660857e6ba9bc55c6279a49a38 Mon Sep 17 00:00:00 2001
From: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
Date: Mon, 3 Apr 2023 15:28:41 +0200
Subject: [PATCH] fix(secrets): Minimize secret holding time (#13002)

---
 plugins/common/kafka/sasl.go                  |  6 ++---
 plugins/inputs/http_response/http_response.go |  9 ++++---
 plugins/inputs/mysql/mysql.go                 | 12 +++++-----
 .../opensearch_query/opensearch_query.go      |  5 ++--
 plugins/inputs/postgresql/service.go          | 15 +++++++-----
 plugins/inputs/snmp_trap/snmp_trap.go         | 24 ++++++++++++-------
 plugins/inputs/sql/sql.go                     | 14 ++++-------
 plugins/inputs/sqlserver/sqlserver.go         | 22 +++++++++--------
 plugins/inputs/vsphere/client.go              |  5 ++--
 .../outputs/elasticsearch/elasticsearch.go    |  9 ++++---
 plugins/outputs/groundwork/groundwork.go      |  5 ++--
 plugins/outputs/http/http.go                  |  6 ++---
 plugins/outputs/influxdb/http.go              |  6 ++---
 plugins/outputs/instrumental/instrumental.go  |  7 +++---
 plugins/outputs/iotdb/iotdb.go                |  6 +++--
 plugins/outputs/loki/loki.go                  |  6 ++---
 16 files changed, 86 insertions(+), 71 deletions(-)

diff --git a/plugins/common/kafka/sasl.go b/plugins/common/kafka/sasl.go
index a28c6014e..56830be3a 100644
--- a/plugins/common/kafka/sasl.go
+++ b/plugins/common/kafka/sasl.go
@@ -32,14 +32,14 @@ func (k *SASLAuth) SetSASLConfig(cfg *sarama.Config) error {
 	if err != nil {
 		return fmt.Errorf("getting username failed: %w", err)
 	}
-	defer config.ReleaseSecret(username)
+	cfg.Net.SASL.User = string(username)
+	config.ReleaseSecret(username)
 	password, err := k.SASLPassword.Get()
 	if err != nil {
 		return fmt.Errorf("getting password failed: %w", err)
 	}
-	defer config.ReleaseSecret(password)
-	cfg.Net.SASL.User = string(username)
 	cfg.Net.SASL.Password = string(password)
+	config.ReleaseSecret(password)
 
 	if k.SASLMechanism != "" {
 		cfg.Net.SASL.Mechanism = sarama.SASLMechanism(k.SASLMechanism)
diff --git a/plugins/inputs/http_response/http_response.go b/plugins/inputs/http_response/http_response.go
index bff0d69c1..44afebb0f 100644
--- a/plugins/inputs/http_response/http_response.go
+++ b/plugins/inputs/http_response/http_response.go
@@ -399,6 +399,10 @@ func (h *HTTPResponse) Gather(acc telegraf.Accumulator) error {
 }
 
 func (h *HTTPResponse) setRequestAuth(request *http.Request) error {
+	if h.Username.Empty() || h.Password.Empty() {
+		return nil
+	}
+
 	username, err := h.Username.Get()
 	if err != nil {
 		return fmt.Errorf("getting username failed: %w", err)
@@ -409,9 +413,8 @@ func (h *HTTPResponse) setRequestAuth(request *http.Request) error {
 		return fmt.Errorf("getting password failed: %w", err)
 	}
 	defer config.ReleaseSecret(password)
-	if len(username) != 0 || len(password) != 0 {
-		request.SetBasicAuth(string(username), string(password))
-	}
+	request.SetBasicAuth(string(username), string(password))
+
 	return nil
 }
 
diff --git a/plugins/inputs/mysql/mysql.go b/plugins/inputs/mysql/mysql.go
index 8e0121c54..a2d263a8f 100644
--- a/plugins/inputs/mysql/mysql.go
+++ b/plugins/inputs/mysql/mysql.go
@@ -105,12 +105,12 @@ func (m *Mysql) Init() error {
 
 	// Adapt the DSN string
 	for i, server := range m.Servers {
-		s, err := server.Get()
+		dsnSecret, err := server.Get()
 		if err != nil {
 			return fmt.Errorf("getting server %d failed: %w", i, err)
 		}
-		dsn := string(s)
-		config.ReleaseSecret(s)
+		dsn := string(dsnSecret)
+		config.ReleaseSecret(dsnSecret)
 		conf, err := mysql.ParseDSN(dsn)
 		if err != nil {
 			return fmt.Errorf("parsing %q failed: %w", dsn, err)
@@ -415,12 +415,12 @@ const (
 )
 
 func (m *Mysql) gatherServer(server *config.Secret, acc telegraf.Accumulator) error {
-	s, err := server.Get()
+	dsnSecret, err := server.Get()
 	if err != nil {
 		return err
 	}
-	dsn := string(s)
-	config.ReleaseSecret(s)
+	dsn := string(dsnSecret)
+	config.ReleaseSecret(dsnSecret)
 	servtag := getDSNTag(dsn)
 
 	db, err := sql.Open("mysql", dsn)
diff --git a/plugins/inputs/opensearch_query/opensearch_query.go b/plugins/inputs/opensearch_query/opensearch_query.go
index 1d00021f0..1a6a43552 100644
--- a/plugins/inputs/opensearch_query/opensearch_query.go
+++ b/plugins/inputs/opensearch_query/opensearch_query.go
@@ -109,18 +109,19 @@ func (o *OpensearchQuery) newClient() error {
 	if err != nil {
 		return fmt.Errorf("getting username failed: %w", err)
 	}
-	defer config.ReleaseSecret(username)
 	password, err := o.Password.Get()
 	if err != nil {
+		config.ReleaseSecret(username)
 		return fmt.Errorf("getting password failed: %w", err)
 	}
-	defer config.ReleaseSecret(password)
 
 	clientConfig := opensearch.Config{
 		Addresses: o.URLs,
 		Username:  string(username),
 		Password:  string(password),
 	}
+	config.ReleaseSecret(username)
+	config.ReleaseSecret(password)
 
 	if o.InsecureSkipVerify {
 		clientConfig.Transport = &http.Transport{
diff --git a/plugins/inputs/postgresql/service.go b/plugins/inputs/postgresql/service.go
index 35d8cc832..07a49d285 100644
--- a/plugins/inputs/postgresql/service.go
+++ b/plugins/inputs/postgresql/service.go
@@ -102,18 +102,21 @@ var socketRegexp = regexp.MustCompile(`/\.s\.PGSQL\.\d+$`)
 
 // Start starts the ServiceInput's service, whatever that may be
 func (p *Service) Start(telegraf.Accumulator) (err error) {
-	addr, err := p.Address.Get()
+	addrSecret, err := p.Address.Get()
 	if err != nil {
 		return fmt.Errorf("getting address failed: %w", err)
 	}
-	defer config.ReleaseSecret(addr)
+	addr := string(addrSecret)
+	defer config.ReleaseSecret(addrSecret)
 
-	if p.Address.Empty() || string(addr) == "localhost" {
-		addr = []byte("host=localhost sslmode=disable")
-		p.Address = config.NewSecret(addr)
+	if p.Address.Empty() || addr == "localhost" {
+		addr = "host=localhost sslmode=disable"
+		if err := p.Address.Set([]byte(addr)); err != nil {
+			return err
+		}
 	}
 
-	connConfig, err := pgx.ParseConfig(string(addr))
+	connConfig, err := pgx.ParseConfig(addr)
 	if err != nil {
 		return err
 	}
diff --git a/plugins/inputs/snmp_trap/snmp_trap.go b/plugins/inputs/snmp_trap/snmp_trap.go
index d5e6b5fe0..11fb9caa1 100644
--- a/plugins/inputs/snmp_trap/snmp_trap.go
+++ b/plugins/inputs/snmp_trap/snmp_trap.go
@@ -171,28 +171,34 @@ func (s *SnmpTrap) Start(acc telegraf.Accumulator) error {
 			return fmt.Errorf("unknown privacy protocol %q", s.PrivProtocol)
 		}
 
-		secname, err := s.SecName.Get()
+		secnameSecret, err := s.SecName.Get()
 		if err != nil {
 			return fmt.Errorf("getting secname failed: %w", err)
 		}
-		privPasswd, err := s.PrivPassword.Get()
+		secname := string(secnameSecret)
+		config.ReleaseSecret(secnameSecret)
+
+		privPasswdSecret, err := s.PrivPassword.Get()
 		if err != nil {
 			return fmt.Errorf("getting secname failed: %w", err)
 		}
-		authPasswd, err := s.AuthPassword.Get()
+		privPasswd := string(privPasswdSecret)
+		config.ReleaseSecret(privPasswdSecret)
+
+		authPasswdSecret, err := s.AuthPassword.Get()
 		if err != nil {
 			return fmt.Errorf("getting secname failed: %w", err)
 		}
+		authPasswd := string(authPasswdSecret)
+		config.ReleaseSecret(authPasswdSecret)
+
 		s.listener.Params.SecurityParameters = &gosnmp.UsmSecurityParameters{
-			UserName:                 string(secname),
+			UserName:                 secname,
 			PrivacyProtocol:          privacyProtocol,
-			PrivacyPassphrase:        string(privPasswd),
-			AuthenticationPassphrase: string(authPasswd),
+			PrivacyPassphrase:        privPasswd,
+			AuthenticationPassphrase: authPasswd,
 			AuthenticationProtocol:   authenticationProtocol,
 		}
-		config.ReleaseSecret(secname)
-		config.ReleaseSecret(privPasswd)
-		config.ReleaseSecret(authPasswd)
 	}
 
 	// wrap the handler, used in unit tests
diff --git a/plugins/inputs/sql/sql.go b/plugins/inputs/sql/sql.go
index b67487e12..8ba809767 100644
--- a/plugins/inputs/sql/sql.go
+++ b/plugins/inputs/sql/sql.go
@@ -358,13 +358,14 @@ func (s *SQL) Start(_ telegraf.Accumulator) error {
 	var err error
 
 	// Connect to the database server
-	dsn, err := s.Dsn.Get()
+	dsnSecret, err := s.Dsn.Get()
 	if err != nil {
 		return fmt.Errorf("getting DSN failed: %w", err)
 	}
-	defer config.ReleaseSecret(dsn)
+	dsn := string(dsnSecret)
+	config.ReleaseSecret(dsnSecret)
 	s.Log.Debug("Connecting...")
-	s.db, err = dbsql.Open(s.driverName, string(dsn))
+	s.db, err = dbsql.Open(s.driverName, dsn)
 	if err != nil {
 		return err
 	}
@@ -472,12 +473,7 @@ func (s *SQL) executeQuery(ctx context.Context, acc telegraf.Accumulator, q Quer
 }
 
 func (s *SQL) checkDSN() error {
-	dsn, err := s.Dsn.Get()
-	if err != nil {
-		return fmt.Errorf("getting DSN failed: %w", err)
-	}
-	defer config.ReleaseSecret(dsn)
-	if len(dsn) == 0 {
+	if s.Dsn.Empty() {
 		return errors.New("missing data source name (DSN) option")
 	}
 	return nil
diff --git a/plugins/inputs/sqlserver/sqlserver.go b/plugins/inputs/sqlserver/sqlserver.go
index b81e983f9..30eb2e737 100644
--- a/plugins/inputs/sqlserver/sqlserver.go
+++ b/plugins/inputs/sqlserver/sqlserver.go
@@ -202,26 +202,28 @@ func (s *SQLServer) Gather(acc telegraf.Accumulator) error {
 	var healthMetrics = make(map[string]*HealthMetric)
 
 	for i, pool := range s.pools {
+		dnsSecret, err := s.Servers[i].Get()
+		if err != nil {
+			acc.AddError(err)
+			continue
+		}
+		dsn := string(dnsSecret)
+		config.ReleaseSecret(dnsSecret)
+
 		for _, query := range s.queries {
 			wg.Add(1)
-			go func(pool *sql.DB, query Query, serverIndex int) {
+			go func(pool *sql.DB, query Query, dsn string) {
 				defer wg.Done()
-				dsn, err := s.Servers[serverIndex].Get()
-				if err != nil {
-					acc.AddError(err)
-					return
-				}
-				defer config.ReleaseSecret(dsn)
-				queryError := s.gatherServer(pool, query, acc, string(dsn))
+				queryError := s.gatherServer(pool, query, acc, dsn)
 
 				if s.HealthMetric {
 					mutex.Lock()
-					s.gatherHealth(healthMetrics, string(dsn), queryError)
+					s.gatherHealth(healthMetrics, dsn, queryError)
 					mutex.Unlock()
 				}
 
 				acc.AddError(queryError)
-			}(pool, query, i)
+			}(pool, query, dsn)
 		}
 	}
 
diff --git a/plugins/inputs/vsphere/client.go b/plugins/inputs/vsphere/client.go
index 273128e09..af1a8c31f 100644
--- a/plugins/inputs/vsphere/client.go
+++ b/plugins/inputs/vsphere/client.go
@@ -141,13 +141,14 @@ func NewClient(ctx context.Context, vSphereURL *url.URL, vs *VSphere) (*Client,
 		if err != nil {
 			return nil, fmt.Errorf("getting username failed: %w", err)
 		}
-		defer config.ReleaseSecret(username)
 		password, err := vs.Password.Get()
 		if err != nil {
+			config.ReleaseSecret(username)
 			return nil, fmt.Errorf("getting password failed: %w", err)
 		}
-		defer config.ReleaseSecret(password)
 		vSphereURL.User = url.UserPassword(string(username), string(password))
+		config.ReleaseSecret(username)
+		config.ReleaseSecret(password)
 	}
 
 	vs.Log.Debugf("Creating client: %s", vSphereURL.Host)
diff --git a/plugins/outputs/elasticsearch/elasticsearch.go b/plugins/outputs/elasticsearch/elasticsearch.go
index 2a2ce2e34..0d5cadafc 100644
--- a/plugins/outputs/elasticsearch/elasticsearch.go
+++ b/plugins/outputs/elasticsearch/elasticsearch.go
@@ -470,14 +470,14 @@ func (a *Elasticsearch) getAuthOptions() ([]elastic.ClientOptionFunc, error) {
 		if err != nil {
 			return nil, fmt.Errorf("getting username failed: %w", err)
 		}
-		defer config.ReleaseSecret(username)
 		password, err := a.Password.Get()
 		if err != nil {
+			config.ReleaseSecret(username)
 			return nil, fmt.Errorf("getting password failed: %w", err)
 		}
-		defer config.ReleaseSecret(password)
-
 		fns = append(fns, elastic.SetBasicAuth(string(username), string(password)))
+		config.ReleaseSecret(username)
+		config.ReleaseSecret(password)
 	}
 
 	if !a.AuthBearerToken.Empty() {
@@ -485,10 +485,9 @@ func (a *Elasticsearch) getAuthOptions() ([]elastic.ClientOptionFunc, error) {
 		if err != nil {
 			return nil, fmt.Errorf("getting token failed: %w", err)
 		}
-		defer config.ReleaseSecret(token)
-
 		auth := []string{"Bearer " + string(token)}
 		fns = append(fns, elastic.SetHeaders(http.Header{"Authorization": auth}))
+		config.ReleaseSecret(token)
 	}
 	return fns, nil
 }
diff --git a/plugins/outputs/groundwork/groundwork.go b/plugins/outputs/groundwork/groundwork.go
index 0f5018bba..ec0162264 100644
--- a/plugins/outputs/groundwork/groundwork.go
+++ b/plugins/outputs/groundwork/groundwork.go
@@ -77,12 +77,11 @@ func (g *Groundwork) Init() error {
 	if err != nil {
 		return fmt.Errorf("getting username failed: %w", err)
 	}
-	defer config.ReleaseSecret(username)
 	password, err := g.Password.Get()
 	if err != nil {
+		config.ReleaseSecret(username)
 		return fmt.Errorf("getting password failed: %w", err)
 	}
-	defer config.ReleaseSecret(password)
 	g.client = clients.GWClient{
 		AppName: "telegraf",
 		AppType: g.DefaultAppType,
@@ -93,6 +92,8 @@ func (g *Groundwork) Init() error {
 			IsDynamicInventory: true,
 		},
 	}
+	config.ReleaseSecret(username)
+	config.ReleaseSecret(password)
 
 	logper.SetLogger(
 		func(fields interface{}, format string, a ...interface{}) {
diff --git a/plugins/outputs/http/http.go b/plugins/outputs/http/http.go
index 39261b6e8..77d55e654 100644
--- a/plugins/outputs/http/http.go
+++ b/plugins/outputs/http/http.go
@@ -183,14 +183,14 @@ func (h *HTTP) writeMetric(reqBody []byte) error {
 		if err != nil {
 			return fmt.Errorf("getting username failed: %w", err)
 		}
-		defer config.ReleaseSecret(username)
 		password, err := h.Password.Get()
 		if err != nil {
+			config.ReleaseSecret(username)
 			return fmt.Errorf("getting password failed: %w", err)
 		}
-		defer config.ReleaseSecret(password)
-
 		req.SetBasicAuth(string(username), string(password))
+		config.ReleaseSecret(username)
+		config.ReleaseSecret(password)
 	}
 
 	// google api auth
diff --git a/plugins/outputs/influxdb/http.go b/plugins/outputs/influxdb/http.go
index 7fcafbe79..e25d62498 100644
--- a/plugins/outputs/influxdb/http.go
+++ b/plugins/outputs/influxdb/http.go
@@ -494,14 +494,14 @@ func (c *httpClient) addHeaders(req *http.Request) error {
 		if err != nil {
 			return fmt.Errorf("getting username failed: %w", err)
 		}
-		defer config.ReleaseSecret(username)
 		password, err := c.config.Password.Get()
 		if err != nil {
+			config.ReleaseSecret(username)
 			return fmt.Errorf("getting password failed: %w", err)
 		}
-		defer config.ReleaseSecret(password)
-
 		req.SetBasicAuth(string(username), string(password))
+		config.ReleaseSecret(username)
+		config.ReleaseSecret(password)
 	}
 
 	for header, value := range c.config.Headers {
diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go
index e30fceb43..9bf24400c 100644
--- a/plugins/outputs/instrumental/instrumental.go
+++ b/plugins/outputs/instrumental/instrumental.go
@@ -165,13 +165,14 @@ func (i *Instrumental) Write(metrics []telegraf.Metric) error {
 }
 
 func (i *Instrumental) authenticate(conn net.Conn) error {
-	token, err := i.APIToken.Get()
+	tokenSecret, err := i.APIToken.Get()
 	if err != nil {
 		return fmt.Errorf("getting token failed: %w", err)
 	}
-	defer config.ReleaseSecret(token)
+	token := string(tokenSecret)
+	config.ReleaseSecret(tokenSecret)
 
-	if _, err := fmt.Fprintf(conn, HandshakeFormat, string(token)); err != nil {
+	if _, err := fmt.Fprintf(conn, HandshakeFormat, token); err != nil {
 		return err
 	}
 
diff --git a/plugins/outputs/iotdb/iotdb.go b/plugins/outputs/iotdb/iotdb.go
index b60cc75c3..0089775f2 100644
--- a/plugins/outputs/iotdb/iotdb.go
+++ b/plugins/outputs/iotdb/iotdb.go
@@ -83,18 +83,20 @@ func (s *IoTDB) Connect() error {
 	if err != nil {
 		return fmt.Errorf("getting username failed: %w", err)
 	}
-	defer config.ReleaseSecret(username)
 	password, err := s.Password.Get()
 	if err != nil {
+		config.ReleaseSecret(username)
 		return fmt.Errorf("getting password failed: %w", err)
 	}
-	defer config.ReleaseSecret(password)
 	sessionConf := &client.Config{
 		Host:     s.Host,
 		Port:     s.Port,
 		UserName: string(username),
 		Password: string(password),
 	}
+	config.ReleaseSecret(username)
+	config.ReleaseSecret(password)
+
 	var ss = client.NewSession(sessionConf)
 	s.session = &ss
 	timeoutInMs := int(time.Duration(s.Timeout).Milliseconds())
diff --git a/plugins/outputs/loki/loki.go b/plugins/outputs/loki/loki.go
index 498c1aa72..0244223b6 100644
--- a/plugins/outputs/loki/loki.go
+++ b/plugins/outputs/loki/loki.go
@@ -158,14 +158,14 @@ func (l *Loki) writeMetrics(s Streams) error {
 		if err != nil {
 			return fmt.Errorf("getting username failed: %w", err)
 		}
-		defer config.ReleaseSecret(username)
 		password, err := l.Password.Get()
 		if err != nil {
+			config.ReleaseSecret(username)
 			return fmt.Errorf("getting password failed: %w", err)
 		}
-		defer config.ReleaseSecret(password)
-
 		req.SetBasicAuth(string(username), string(password))
+		config.ReleaseSecret(password)
+		config.ReleaseSecret(username)
 	}
 
 	for k, v := range l.Headers {