Redact sensative information in Trident logs for Sofidfire and E-Series

This PR addresses the issue of sensitive information exposure in the Trident logs. By implementing the stringer interface for the drivers in a manner that is developer-friendly and easy to maintain provides an easy solution to hide any sensitive information in the Trident logs and other places where backend are printed.
NetApp · Jul 29, 2020 · 4da6ca9 · 4da6ca9
1 parent e6c0251
commit 4da6ca9
Show file tree

Hide file tree

Showing 6 changed files with 334 additions and 3 deletions.
diff --git a/storage/backend.go b/storage/backend.go
@@ -169,6 +169,10 @@ func NewFailedStorageBackend(driver Driver) *Backend {
 	return &backend
 }
 
+func (b Backend) String() string {
+	return fmt.Sprintf("%#v", b)
+}
+
 func (b *Backend) AddStoragePool(pool *Pool) {
 	b.Storage[pool.Name] = pool
 }

diff --git a/storage_drivers/eseries/eseries_iscsi.go b/storage_drivers/eseries/eseries_iscsi.go
@@ -1198,9 +1198,9 @@ func (d *SANStorageDriver) GetExternalConfig() interface{} {
 	// Clone the config so we don't risk altering the original
 	var cloneConfig drivers.ESeriesStorageDriverConfig
 	drivers.Clone(d.Config, &cloneConfig)
-	cloneConfig.Username = ""      // redact the username
-	cloneConfig.Password = ""      // redact the password
-	cloneConfig.PasswordArray = "" // redact the password
+	cloneConfig.Username = "<REDACTED>"      // redact the username
+	cloneConfig.Password = "<REDACTED>"      // redact the password
+	cloneConfig.PasswordArray = "<REDACTED>" // redact the password
 	return cloneConfig
 }
 
@@ -1255,6 +1255,12 @@ func (d *SANStorageDriver) GetVolumeExternal(name string) (*storage.VolumeExtern
 	return d.getVolumeExternal(&volumeAttrs), nil
 }
 
+// Implement stringer interface for the E-Series driver
+func (d SANStorageDriver) String() string {
+	sensitive := d.Config.DebugTraceFlags["sensitive"]
+	return drivers.ToString(sensitive, &d, []string{"API"}, d.GetExternalConfig())
+}
+
 // GetVolumeExternalWrappers queries the storage backend for all relevant info about
 // container volumes managed by this driver.  It then writes a VolumeExternal
 // representation of each volume to the supplied channel, closing the channel

diff --git a/storage_drivers/eseries/eseries_iscsi_test.go b/storage_drivers/eseries/eseries_iscsi_test.go
@@ -0,0 +1,120 @@
+// Copyright 2020 NetApp, Inc. All Rights Reserved.
+
+package eseries
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	drivers "github.com/netapp/trident/storage_drivers"
+	"github.com/netapp/trident/storage_drivers/eseries/api"
+)
+
+const (
+	Username      = "tester"
+	Password      = "password"
+	PasswordArray = "Passwords"
+)
+
+func newTestEseriesSANDriver(showSensitive *bool) *SANStorageDriver {
+	config := &drivers.ESeriesStorageDriverConfig{}
+	sp := func(s string) *string { return &s }
+
+	config.CommonStorageDriverConfig = &drivers.CommonStorageDriverConfig{}
+	config.CommonStorageDriverConfig.DebugTraceFlags = make(map[string]bool)
+	config.CommonStorageDriverConfig.DebugTraceFlags["method"] = true
+
+	if showSensitive != nil {
+		config.CommonStorageDriverConfig.DebugTraceFlags["sensitive"] = *showSensitive
+	}
+
+	config.Username = Username
+	config.Password = Password
+	config.PasswordArray = PasswordArray
+	config.WebProxyHostname = "10.0.0.1"
+	config.WebProxyPort = "2222"
+	config.WebProxyUseHTTP = false
+	config.WebProxyVerifyTLS = false
+	config.ControllerA = "10.0.0.2"
+	config.ControllerB = "10.0.0.3"
+	config.HostDataIP = "10.0.0.4"
+	config.StorageDriverName = "eseries-san"
+	config.StoragePrefix = sp("test_")
+
+	telemetry := make(map[string]string)
+	telemetry["version"] = "20.07.0"
+	telemetry["plugin"] = "eseries"
+	telemetry["storagePrefix"] = *config.StoragePrefix
+
+	API := api.NewAPIClient(api.ClientConfig{
+		WebProxyHostname:      config.WebProxyHostname,
+		WebProxyPort:          config.WebProxyPort,
+		WebProxyUseHTTP:       config.WebProxyUseHTTP,
+		WebProxyVerifyTLS:     config.WebProxyVerifyTLS,
+		Username:              config.Username,
+		Password:              config.Password,
+		ControllerA:           config.ControllerA,
+		ControllerB:           config.ControllerB,
+		PasswordArray:         config.PasswordArray,
+		PoolNameSearchPattern: config.PoolNameSearchPattern,
+		HostDataIP:            config.HostDataIP,
+		Protocol:              "iscsi",
+		AccessGroup:           config.AccessGroup,
+		HostType:              config.HostType,
+		DriverName:            "eseries-iscsi",
+		Telemetry:             telemetry,
+	})
+
+	sanDriver := &SANStorageDriver{}
+	sanDriver.Config = *config
+	sanDriver.API = API
+
+	return sanDriver
+}
+
+func TestEseriesSANStorageDriverConfigString(t *testing.T) {
+
+	var EseriesSANDrivers = []SANStorageDriver{
+		*newTestEseriesSANDriver(&[]bool{true}[0]),
+		*newTestEseriesSANDriver(&[]bool{false}[0]),
+		*newTestEseriesSANDriver(nil),
+	}
+
+	for _, EseriesSANDriver := range EseriesSANDrivers {
+		sensitive, ok := EseriesSANDriver.Config.DebugTraceFlags["sensitive"]
+
+		switch {
+
+		case !ok:
+			assert.Contains(t, EseriesSANDriver.String(), "<REDACTED>",
+				"Eseries driver does not contain <REDACTED>")
+			assert.Contains(t, EseriesSANDriver.String(), "API:<REDACTED>",
+				"Eseries driver does not redact API information")
+			assert.NotContains(t, EseriesSANDriver.String(), Username,
+				"Eseries driver contains  username")
+			assert.NotContains(t, EseriesSANDriver.String(), Password,
+				"Eseries driver contains password")
+			assert.NotContains(t, EseriesSANDriver.String(), PasswordArray,
+				"Eseries driver contains password array")
+		case ok && sensitive:
+			assert.Contains(t, EseriesSANDriver.String(), Username,
+				"Eseries driver does not contain username")
+			assert.Contains(t, EseriesSANDriver.String(), Password,
+				"Eseries driver does not contain password")
+			assert.Contains(t, EseriesSANDriver.String(), PasswordArray,
+				"Eseries driver does not contain password array")
+		case ok && !sensitive:
+			assert.Contains(t, EseriesSANDriver.String(), "<REDACTED>",
+				"Eseries driver does not contain <REDACTED>")
+			assert.Contains(t, EseriesSANDriver.String(), "API:<REDACTED>",
+				"Eseries driver does not redact API information")
+			assert.NotContains(t, EseriesSANDriver.String(), Username,
+				"Eseries driver contains  username")
+			assert.NotContains(t, EseriesSANDriver.String(), Password,
+				"Eseries driver contains password")
+			assert.NotContains(t, EseriesSANDriver.String(), PasswordArray,
+				"Eseries driver contains password array")
+		}
+	}
+}
diff --git a/storage_drivers/solidfire/solidfire_san.go b/storage_drivers/solidfire/solidfire_san.go
@@ -1716,6 +1716,12 @@ func (d *SANStorageDriver) GetVolumeExternal(name string) (*storage.VolumeExtern
 	return d.getVolumeExternal(name, &volume), nil
 }
 
+// Implement stringer interface for the SANStorageDriver driver
+func (d SANStorageDriver) String() string {
+	sensitive := d.Config.DebugTraceFlags["sensitive"]
+	return drivers.ToString(sensitive, &d, []string{"Client", "AccountID"}, d.GetExternalConfig())
+}
+
 // GetVolumeExternalWrappers queries the storage backend for all relevant info about
 // container volumes managed by this driver.  It then writes a VolumeExternal
 // representation of each volume to the supplied channel, closing the channel

diff --git a/storage_drivers/solidfire/solidfire_san_test.go b/storage_drivers/solidfire/solidfire_san_test.go
@@ -0,0 +1,135 @@
+// Copyright 2020 NetApp, Inc. All Rights Reserved.
+
+package solidfire
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	drivers "github.com/netapp/trident/storage_drivers"
+	"github.com/netapp/trident/storage_drivers/solidfire/api"
+)
+
+const (
+	TenantName = "tester"
+	AdminPass  = "admin:password"
+	Endpoint   = "https://" + AdminPass + "@10.0.0.1/json-rpc/7.0"
+)
+
+func newTestSolidfireSANDriver(showSensitive *bool) *SANStorageDriver {
+	config := &drivers.SolidfireStorageDriverConfig{}
+	sp := func(s string) *string { return &s }
+
+	config.CommonStorageDriverConfig = &drivers.CommonStorageDriverConfig{}
+	config.CommonStorageDriverConfig.DebugTraceFlags = make(map[string]bool)
+	config.CommonStorageDriverConfig.DebugTraceFlags["method"] = true
+
+	if showSensitive != nil {
+		config.CommonStorageDriverConfig.DebugTraceFlags["sensitive"] = *showSensitive
+	}
+
+	config.TenantName = TenantName
+	config.EndPoint = Endpoint
+	config.SVIP = "10.0.0.1:1000"
+	config.InitiatorIFace = "default"
+	config.Types = &[]api.VolType{
+		{
+			Type: "Gold",
+			QOS: api.QoS{
+				BurstIOPS: 10000,
+				MaxIOPS:   8000,
+				MinIOPS:   6000,
+			},
+		},
+		{
+			Type: "Bronze",
+			QOS: api.QoS{
+				BurstIOPS: 4000,
+				MaxIOPS:   2000,
+				MinIOPS:   1000,
+			},
+		},
+	}
+	config.AccessGroups = []int64{}
+	config.UseCHAP = true
+	config.DefaultBlockSize = 4096
+	config.StorageDriverName = "solidfire-san"
+	config.StoragePrefix = sp("test_")
+
+	cfg := api.Config{
+		TenantName:       config.TenantName,
+		EndPoint:         Endpoint,
+		SVIP:             config.SVIP,
+		InitiatorIFace:   config.InitiatorIFace,
+		Types:            config.Types,
+		LegacyNamePrefix: config.LegacyNamePrefix,
+		AccessGroups:     config.AccessGroups,
+		DefaultBlockSize: 4096,
+		DebugTraceFlags:  config.DebugTraceFlags,
+	}
+
+	client, _ := api.NewFromParameters(Endpoint, config.SVIP, cfg)
+
+	sanDriver := &SANStorageDriver{}
+	sanDriver.Config = *config
+	sanDriver.Client = client
+	sanDriver.AccountID = 2222
+	sanDriver.AccessGroups = []int64{}
+	sanDriver.LegacyNamePrefix = "oldtest_"
+	sanDriver.InitiatorIFace = "default"
+	sanDriver.DefaultMaxIOPS = 20000
+	sanDriver.DefaultMinIOPS = 1000
+
+	return sanDriver
+}
+
+func TestSolidfireSANStorageDriverConfigString(t *testing.T) {
+
+	var solidfireSANDrivers = []SANStorageDriver{
+		*newTestSolidfireSANDriver(&[]bool{true}[0]),
+		*newTestSolidfireSANDriver(&[]bool{false}[0]),
+		*newTestSolidfireSANDriver(nil),
+	}
+
+	for _, solidfireSANDriver := range solidfireSANDrivers {
+		sensitive, ok := solidfireSANDriver.Config.DebugTraceFlags["sensitive"]
+
+		switch {
+
+		case !ok:
+			assert.Contains(t, solidfireSANDriver.String(), "<REDACTED>",
+				"Solidfire driver does not contain <REDACTED>")
+			assert.Contains(t, solidfireSANDriver.String(), "Client:<REDACTED>",
+				"Solidfire driver does not redact client API information")
+			assert.Contains(t, solidfireSANDriver.String(), "AccountID:<REDACTED>",
+				"Solidfire driver does not redact Account ID information")
+			assert.NotContains(t, solidfireSANDriver.String(), TenantName,
+				"Solidfire driver contains tenant name")
+			assert.NotContains(t, solidfireSANDriver.String(), AdminPass,
+				"Solidfire driver contains endpoint's admin and password")
+			assert.NotContains(t, solidfireSANDriver.String(), "2222",
+				"Solidfire driver contains Account ID")
+		case ok && sensitive:
+			assert.Contains(t, solidfireSANDriver.String(), TenantName,
+				"Solidfire driver does not contain tenant name")
+			assert.Contains(t, solidfireSANDriver.String(), AdminPass,
+				"Solidfire driver does not contain endpoint's admin and password")
+			assert.Contains(t, solidfireSANDriver.String(), "2222",
+				"Solidfire driver does not contain Account ID")
+		case ok && !sensitive:
+			assert.Contains(t, solidfireSANDriver.String(), "<REDACTED>",
+				"Solidfire driver does not contain <REDACTED>")
+			assert.Contains(t, solidfireSANDriver.String(), "Client:<REDACTED>",
+				"Solidfire driver does not redact client API information")
+			assert.Contains(t, solidfireSANDriver.String(), "AccountID:<REDACTED>",
+				"Solidfire driver does not redact Account ID information")
+			assert.NotContains(t, solidfireSANDriver.String(), TenantName,
+				"Solidfire driver contains tenant name")
+			assert.NotContains(t, solidfireSANDriver.String(), AdminPass,
+				"Solidfire driver contains endpoint's admin and password")
+			assert.NotContains(t, solidfireSANDriver.String(), "2222",
+				"Solidfire driver contains Account ID")
+		}
+	}
+}