diff --git a/conf/config.toml b/conf/config.toml index 20f664a4c85..438d2c857a5 100644 --- a/conf/config.toml +++ b/conf/config.toml @@ -33,7 +33,10 @@ # key-path = "" ## A CN which must be provided by a client # cert-allowed-cn = ["example.com"] -## Whether or not to enable redact log. +## Whether to enable the log redaction. It can be the following values: +## - false: disable redact log. +## - true: enable redact log, which will replace the sensitive information with "?". +## - "MARKER": enable redact log, which will use single guillemets ‹› to enclose the sensitive information. # redact-info-log = false [security.encryption] diff --git a/pkg/core/region.go b/pkg/core/region.go index c83f4387eab..73f2fdd62e7 100644 --- a/pkg/core/region.go +++ b/pkg/core/region.go @@ -2049,14 +2049,6 @@ func DiffRegionKeyInfo(origin *RegionInfo, other *RegionInfo) string { return strings.Join(ret, ", ") } -// String converts slice of bytes to string without copy. -func String(b []byte) string { - if len(b) == 0 { - return "" - } - return unsafe.String(unsafe.SliceData(b), len(b)) -} - // ToUpperASCIIInplace bytes.ToUpper but zero-cost func ToUpperASCIIInplace(s []byte) []byte { hasLower := false @@ -2095,7 +2087,7 @@ func HexRegionKey(key []byte) []byte { // HexRegionKeyStr converts region key to hex format. Used for formatting region in // logs. func HexRegionKeyStr(key []byte) string { - return String(HexRegionKey(key)) + return typeutil.BytesToString(HexRegionKey(key)) } // RegionToHexMeta converts a region meta's keys to hex format. Used for formatting diff --git a/pkg/storage/hot_region_storage.go b/pkg/storage/hot_region_storage.go index c08825dbba1..50fa7455f44 100644 --- a/pkg/storage/hot_region_storage.go +++ b/pkg/storage/hot_region_storage.go @@ -37,6 +37,7 @@ import ( "github.com/tikv/pd/pkg/storage/kv" "github.com/tikv/pd/pkg/utils/logutil" "github.com/tikv/pd/pkg/utils/syncutil" + "github.com/tikv/pd/pkg/utils/typeutil" "go.uber.org/zap" ) @@ -266,8 +267,8 @@ func (h *HotRegionStorage) packHistoryHotRegions(historyHotRegions []HistoryHotR if err != nil { return err } - historyHotRegions[i].StartKey = core.String(region.StartKey) - historyHotRegions[i].EndKey = core.String(region.EndKey) + historyHotRegions[i].StartKey = typeutil.BytesToString(region.StartKey) + historyHotRegions[i].EndKey = typeutil.BytesToString(region.EndKey) key := HotRegionStorePath(hotRegionType, historyHotRegions[i].UpdateTime, historyHotRegions[i].RegionID) h.batchHotInfo[key] = &historyHotRegions[i] } @@ -385,8 +386,8 @@ func (it *HotRegionStorageIterator) Next() (*HistoryHotRegion, error) { if err := encryption.DecryptRegion(region, it.encryptionKeyManager); err != nil { return nil, err } - message.StartKey = core.String(region.StartKey) - message.EndKey = core.String(region.EndKey) + message.StartKey = typeutil.BytesToString(region.StartKey) + message.EndKey = typeutil.BytesToString(region.EndKey) message.EncryptionMeta = nil return &message, nil } diff --git a/pkg/utils/configutil/configutil.go b/pkg/utils/configutil/configutil.go index 086f74ff842..48be7ff8c02 100644 --- a/pkg/utils/configutil/configutil.go +++ b/pkg/utils/configutil/configutil.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/pflag" "github.com/tikv/pd/pkg/encryption" "github.com/tikv/pd/pkg/utils/grpcutil" + "github.com/tikv/pd/pkg/utils/logutil" "github.com/tikv/pd/pkg/utils/typeutil" ) @@ -78,9 +79,12 @@ func (m *ConfigMetaData) CheckUndecoded() error { // SecurityConfig indicates the security configuration type SecurityConfig struct { grpcutil.TLSConfig - // RedactInfoLog indicates that whether enabling redact log - RedactInfoLog bool `toml:"redact-info-log" json:"redact-info-log"` - Encryption encryption.Config `toml:"encryption" json:"encryption"` + // RedactInfoLog indicates that whether to enable the log redaction. It can be the following values: + // - false: disable redact log. + // - true: enable redact log, which will replace the sensitive information with "?". + // - "MARKER": enable redact log, which will use single guillemets ‹› to enclose the sensitive information. + RedactInfoLog logutil.RedactInfoLogType `toml:"redact-info-log" json:"redact-info-log"` + Encryption encryption.Config `toml:"encryption" json:"encryption"` } // PrintConfigCheckMsg prints the message about configuration checks. diff --git a/pkg/utils/logutil/log.go b/pkg/utils/logutil/log.go index ff6ffa7af9a..c7a9ac2f3b7 100644 --- a/pkg/utils/logutil/log.go +++ b/pkg/utils/logutil/log.go @@ -15,12 +15,15 @@ package logutil import ( + "encoding/json" + "errors" "fmt" "strings" "sync/atomic" "github.com/pingcap/log" "github.com/tikv/pd/pkg/errs" + "github.com/tikv/pd/pkg/utils/typeutil" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -67,16 +70,19 @@ func StringToZapLogLevel(level string) zapcore.Level { } // SetupLogger setup the logger. -func SetupLogger(logConfig log.Config, logger **zap.Logger, logProps **log.ZapProperties, enabled ...bool) error { +func SetupLogger( + logConfig log.Config, + logger **zap.Logger, + logProps **log.ZapProperties, + redactInfoLogType RedactInfoLogType, +) error { lg, p, err := log.InitLogger(&logConfig, zap.AddStacktrace(zapcore.FatalLevel)) if err != nil { return errs.ErrInitLogger.Wrap(err) } *logger = lg *logProps = p - if len(enabled) > 0 { - SetRedactLog(enabled[0]) - } + setRedactType(redactInfoLogType) return nil } @@ -88,22 +94,111 @@ func LogPanic() { } } +// RedactInfoLogType is the behavior of redacting sensitive information in logs. +type RedactInfoLogType int + +const ( + // RedactInfoLogOFF means log redaction is disabled. + RedactInfoLogOFF RedactInfoLogType = iota + // RedactInfoLogON means log redaction is enabled, and will replace the sensitive information with "?". + RedactInfoLogON + // RedactInfoLogMarker means log redaction is enabled, and will use single guillemets ‹› to enclose the sensitive information. + RedactInfoLogMarker +) + +// MarshalJSON implements the `json.Marshaler` interface to ensure the compatibility. +func (t RedactInfoLogType) MarshalJSON() ([]byte, error) { + switch t { + case RedactInfoLogON: + return json.Marshal(true) + case RedactInfoLogMarker: + return json.Marshal("MARKER") + default: + } + return json.Marshal(false) +} + +const invalidRedactInfoLogTypeErrMsg = `the "redact-info-log" value is invalid; it should be either false, true, or "MARKER"` + +// UnmarshalJSON implements the `json.Marshaler` interface to ensure the compatibility. +func (t *RedactInfoLogType) UnmarshalJSON(data []byte) error { + var s string + err := json.Unmarshal(data, &s) + if err == nil && strings.ToUpper(s) == "MARKER" { + *t = RedactInfoLogMarker + return nil + } + var b bool + err = json.Unmarshal(data, &b) + if err != nil { + return errors.New(invalidRedactInfoLogTypeErrMsg) + } + if b { + *t = RedactInfoLogON + } else { + *t = RedactInfoLogOFF + } + return nil +} + +// UnmarshalTOML implements the `toml.Unmarshaler` interface to ensure the compatibility. +func (t *RedactInfoLogType) UnmarshalTOML(data any) error { + switch v := data.(type) { + case bool: + if v { + *t = RedactInfoLogON + } else { + *t = RedactInfoLogOFF + } + return nil + case string: + if strings.ToUpper(v) == "MARKER" { + *t = RedactInfoLogMarker + return nil + } + return errors.New(invalidRedactInfoLogTypeErrMsg) + default: + } + return errors.New(invalidRedactInfoLogTypeErrMsg) +} + var ( - enabledRedactLog atomic.Value + curRedactType atomic.Value ) func init() { - SetRedactLog(false) + setRedactType(RedactInfoLogOFF) +} + +func getRedactType() RedactInfoLogType { + return curRedactType.Load().(RedactInfoLogType) } -// IsRedactLogEnabled indicates whether the log desensitization is enabled -func IsRedactLogEnabled() bool { - return enabledRedactLog.Load().(bool) +func setRedactType(redactInfoLogType RedactInfoLogType) { + curRedactType.Store(redactInfoLogType) } -// SetRedactLog sets enabledRedactLog -func SetRedactLog(enabled bool) { - enabledRedactLog.Store(enabled) +const ( + leftMark = '‹' + rightMark = '›' +) + +func redactInfo(input string) string { + res := &strings.Builder{} + res.Grow(len(input) + 2) + _, _ = res.WriteRune(leftMark) + for _, c := range input { + // Double the mark character if it is already in the input string. + // to avoid the ambiguity of the redacted content. + if c == leftMark || c == rightMark { + _, _ = res.WriteRune(c) + _, _ = res.WriteRune(c) + } else { + _, _ = res.WriteRune(c) + } + } + _, _ = res.WriteRune(rightMark) + return res.String() } // ZapRedactByteString receives []byte argument and return omitted information zap.Field if redact log enabled @@ -123,34 +218,48 @@ func ZapRedactStringer(key string, arg fmt.Stringer) zap.Field { // RedactBytes receives []byte argument and return omitted information if redact log enabled func RedactBytes(arg []byte) []byte { - if IsRedactLogEnabled() { + switch getRedactType() { + case RedactInfoLogON: return []byte("?") + case RedactInfoLogMarker: + // Use unsafe conversion to avoid copy. + return typeutil.StringToBytes(redactInfo(typeutil.BytesToString(arg))) + default: } return arg } // RedactString receives string argument and return omitted information if redact log enabled func RedactString(arg string) string { - if IsRedactLogEnabled() { + switch getRedactType() { + case RedactInfoLogON: return "?" + case RedactInfoLogMarker: + return redactInfo(arg) + default: } return arg } // RedactStringer receives stringer argument and return omitted information if redact log enabled func RedactStringer(arg fmt.Stringer) fmt.Stringer { - if IsRedactLogEnabled() { - return stringer{} + switch getRedactType() { + case RedactInfoLogON: + return &redactedStringer{"?"} + case RedactInfoLogMarker: + return &redactedStringer{redactInfo(arg.String())} + default: } return arg } -type stringer struct { +type redactedStringer struct { + content string } // String implement fmt.Stringer -func (stringer) String() string { - return "?" +func (rs *redactedStringer) String() string { + return rs.content } // CondUint32 constructs a field with the given key and value conditionally. diff --git a/pkg/utils/logutil/log_test.go b/pkg/utils/logutil/log_test.go index 650ba62fe9d..ae0534bbc7a 100644 --- a/pkg/utils/logutil/log_test.go +++ b/pkg/utils/logutil/log_test.go @@ -15,9 +15,11 @@ package logutil import ( - "fmt" + "encoding/json" + "strings" "testing" + "github.com/BurntSushi/toml" "github.com/stretchr/testify/require" "go.uber.org/zap/zapcore" ) @@ -33,52 +35,185 @@ func TestStringToZapLogLevel(t *testing.T) { re.Equal(zapcore.InfoLevel, StringToZapLogLevel("whatever")) } +func TestRedactInfoLogType(t *testing.T) { + re := require.New(t) + // JSON unmarshal. + jsonUnmarshalTestCases := []struct { + jsonStr string + expect RedactInfoLogType + expectErr bool + }{ + {`false`, RedactInfoLogOFF, false}, + {`true`, RedactInfoLogON, false}, + {`"MARKER"`, RedactInfoLogMarker, false}, + {`"marker"`, RedactInfoLogMarker, false}, + {`"OTHER"`, RedactInfoLogOFF, true}, + {`"OFF"`, RedactInfoLogOFF, true}, + {`"ON"`, RedactInfoLogOFF, true}, + {`"off"`, RedactInfoLogOFF, true}, + {`"on"`, RedactInfoLogOFF, true}, + {`""`, RedactInfoLogOFF, true}, + {`"fALSe"`, RedactInfoLogOFF, true}, + {`"trUE"`, RedactInfoLogOFF, true}, + } + var redactType RedactInfoLogType + for idx, tc := range jsonUnmarshalTestCases { + t.Logf("test case %d: %s", idx, tc.jsonStr) + err := json.Unmarshal([]byte(tc.jsonStr), &redactType) + if tc.expectErr { + re.Error(err) + re.ErrorContains(err, invalidRedactInfoLogTypeErrMsg) + } else { + re.NoError(err) + re.Equal(tc.expect, redactType) + } + } + // JSON marshal. + jsonMarshalTestCases := []struct { + typ RedactInfoLogType + expect string + }{ + {RedactInfoLogOFF, `false`}, + {RedactInfoLogON, `true`}, + {RedactInfoLogMarker, `"MARKER"`}, + } + for _, tc := range jsonMarshalTestCases { + b, err := json.Marshal(tc.typ) + re.NoError(err) + re.Equal(tc.expect, string(b)) + } + // TOML unmarshal. + tomlTestCases := []struct { + tomlStr string + expect RedactInfoLogType + expectErr bool + }{ + {`redact-info-log = false`, RedactInfoLogOFF, false}, + {`redact-info-log = true`, RedactInfoLogON, false}, + {`redact-info-log = "MARKER"`, RedactInfoLogMarker, false}, + {`redact-info-log = "marker"`, RedactInfoLogMarker, false}, + {`redact-info-log = "OTHER"`, RedactInfoLogOFF, true}, + {`redact-info-log = "OFF"`, RedactInfoLogOFF, true}, + {`redact-info-log = "ON"`, RedactInfoLogOFF, true}, + {`redact-info-log = "off"`, RedactInfoLogOFF, true}, + {`redact-info-log = "on"`, RedactInfoLogOFF, true}, + {`redact-info-log = ""`, RedactInfoLogOFF, true}, + {`redact-info-log = "fALSe"`, RedactInfoLogOFF, true}, + {`redact-info-log = "trUE"`, RedactInfoLogOFF, true}, + } + var config struct { + RedactInfoLog RedactInfoLogType `toml:"redact-info-log"` + } + for _, tc := range tomlTestCases { + _, err := toml.Decode(tc.tomlStr, &config) + if tc.expectErr { + re.Error(err) + re.ErrorContains(err, invalidRedactInfoLogTypeErrMsg) + } else { + re.NoError(err) + re.Equal(tc.expect, config.RedactInfoLog) + } + } +} + func TestRedactLog(t *testing.T) { re := require.New(t) testCases := []struct { - name string - arg any - enableRedactLog bool - expect any + name string + arg any + redactInfoLogType RedactInfoLogType + expect any }{ { - name: "string arg, enable redact", - arg: "foo", - enableRedactLog: true, - expect: "?", + name: "string arg, enable redact", + arg: "foo", + redactInfoLogType: RedactInfoLogON, + expect: "?", + }, + { + name: "string arg", + arg: "foo", + redactInfoLogType: RedactInfoLogOFF, + expect: "foo", + }, + { + name: "[]byte arg, enable redact", + arg: []byte("foo"), + redactInfoLogType: RedactInfoLogON, + expect: []byte("?"), + }, + { + name: "[]byte arg", + arg: []byte("foo"), + redactInfoLogType: RedactInfoLogOFF, + expect: []byte("foo"), + }, + { + name: "string arg, enable redact marker", + arg: "foo", + redactInfoLogType: RedactInfoLogMarker, + expect: "‹foo›", + }, + { + name: "string arg contains left marker, enable redact marker", + arg: "f‹oo", + redactInfoLogType: RedactInfoLogMarker, + expect: "‹f‹‹oo›", + }, + { + name: "string arg contains right marker, enable redact marker", + arg: "foo›", + redactInfoLogType: RedactInfoLogMarker, + expect: "‹foo›››", + }, + { + name: "string arg contains marker, enable redact marker", + arg: "f‹oo›", + redactInfoLogType: RedactInfoLogMarker, + expect: "‹f‹‹oo›››", + }, + { + name: "[]byte arg, enable redact marker", + arg: []byte("foo"), + redactInfoLogType: RedactInfoLogMarker, + expect: []byte("‹foo›"), }, { - name: "string arg", - arg: "foo", - enableRedactLog: false, - expect: "foo", + name: "[]byte arg contains left marker, enable redact marker", + arg: []byte("foo‹"), + redactInfoLogType: RedactInfoLogMarker, + expect: []byte("‹foo‹‹›"), }, { - name: "[]byte arg, enable redact", - arg: []byte("foo"), - enableRedactLog: true, - expect: []byte("?"), + name: "[]byte arg contains right marker, enable redact marker", + arg: []byte("›foo"), + redactInfoLogType: RedactInfoLogMarker, + expect: []byte("‹››foo›"), }, { - name: "[]byte arg", - arg: []byte("foo"), - enableRedactLog: false, - expect: []byte("foo"), + name: "[]byte arg contains marker, enable redact marker", + arg: []byte("f›o‹o"), + redactInfoLogType: RedactInfoLogMarker, + expect: []byte("‹f››o‹‹o›"), }, } for _, testCase := range testCases { - t.Log(testCase.name) - SetRedactLog(testCase.enableRedactLog) + setRedactType(testCase.redactInfoLogType) + // Create `fmt.Stringer`s to test `RedactStringer` later. + var argStringer, expectStringer = &strings.Builder{}, &strings.Builder{} switch r := testCase.arg.(type) { case []byte: - re.Equal(testCase.expect, RedactBytes(r)) + re.Equal(testCase.expect, RedactBytes(r), testCase.name) + argStringer.Write((testCase.arg).([]byte)) + expectStringer.Write((testCase.expect).([]byte)) case string: - re.Equal(testCase.expect, RedactString(r)) - case fmt.Stringer: - re.Equal(testCase.expect, RedactStringer(r)) + re.Equal(testCase.expect, RedactString(r), testCase.name) + argStringer.WriteString((testCase.arg).(string)) + expectStringer.WriteString((testCase.expect).(string)) default: - panic("unmatched case") + re.FailNow("unmatched case", testCase.name) } + re.Equal(expectStringer.String(), RedactStringer(argStringer).String(), testCase.name) } } diff --git a/pkg/utils/typeutil/conversion.go b/pkg/utils/typeutil/conversion.go index 128c7a887a4..dab12a52d9e 100644 --- a/pkg/utils/typeutil/conversion.go +++ b/pkg/utils/typeutil/conversion.go @@ -16,6 +16,7 @@ package typeutil import ( "encoding/binary" + "unsafe" "github.com/tikv/pd/pkg/errs" ) @@ -68,3 +69,19 @@ func JSONToUint64Slice(from any) ([]uint64, bool) { } return to, true } + +// BytesToString converts slice of bytes to string without copy. +func BytesToString(b []byte) string { + if len(b) == 0 { + return "" + } + return unsafe.String(unsafe.SliceData(b), len(b)) +} + +// StringToBytes converts string to slice of bytes without copy. +func StringToBytes(s string) []byte { + if len(s) == 0 { + return nil + } + return unsafe.Slice(unsafe.StringData(s), len(s)) +} diff --git a/pkg/utils/typeutil/conversion_test.go b/pkg/utils/typeutil/conversion_test.go index 7b17cfcbe2c..e69eeb57e23 100644 --- a/pkg/utils/typeutil/conversion_test.go +++ b/pkg/utils/typeutil/conversion_test.go @@ -73,3 +73,17 @@ func TestJSONToUint64Slice(t *testing.T) { re.False(ok) re.Nil(res) } + +func TestBytesToString(t *testing.T) { + re := require.New(t) + str := "hello" + b := []byte(str) + re.Equal(str, BytesToString(b)) +} + +func TestStringToBytes(t *testing.T) { + re := require.New(t) + str := "hello" + b := StringToBytes(str) + re.Equal([]byte(str), b) +} diff --git a/server/config/config_test.go b/server/config/config_test.go index d7abfe0746a..df23241b787 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -30,12 +30,13 @@ import ( sc "github.com/tikv/pd/pkg/schedule/config" "github.com/tikv/pd/pkg/storage" "github.com/tikv/pd/pkg/utils/configutil" + "github.com/tikv/pd/pkg/utils/logutil" ) func TestSecurity(t *testing.T) { re := require.New(t) cfg := NewConfig() - re.False(cfg.Security.RedactInfoLog) + re.Equal(logutil.RedactInfoLogOFF, cfg.Security.RedactInfoLog) } func TestTLS(t *testing.T) { diff --git a/tests/testutil.go b/tests/testutil.go index 06e15fa2c8a..554def91f08 100644 --- a/tests/testutil.go +++ b/tests/testutil.go @@ -91,10 +91,10 @@ func SetRangePort(start, end int) { var once sync.Once // InitLogger initializes the logger for test. -func InitLogger(logConfig log.Config, logger *zap.Logger, logProps *log.ZapProperties, isRedactInfoLogEnabled bool) (err error) { +func InitLogger(logConfig log.Config, logger *zap.Logger, logProps *log.ZapProperties, redactInfoLog logutil.RedactInfoLogType) (err error) { once.Do(func() { // Setup the logger. - err = logutil.SetupLogger(logConfig, &logger, &logProps, isRedactInfoLogEnabled) + err = logutil.SetupLogger(logConfig, &logger, &logProps, redactInfoLog) if err != nil { return } diff --git a/tools/pd-api-bench/main.go b/tools/pd-api-bench/main.go index f9feeeea580..d62d83437b6 100644 --- a/tools/pd-api-bench/main.go +++ b/tools/pd-api-bench/main.go @@ -92,7 +92,7 @@ func main() { default: log.Fatal("parse cmd flags error", zap.Error(err)) } - err = logutil.SetupLogger(cfg.Log, &cfg.Logger, &cfg.LogProps) + err = logutil.SetupLogger(cfg.Log, &cfg.Logger, &cfg.LogProps, logutil.RedactInfoLogOFF) if err == nil { log.ReplaceGlobals(cfg.Logger, cfg.LogProps) } else { @@ -199,9 +199,7 @@ func parseCaseNameAndConfig(str string) (string, *cases.Config) { strsb := strings.Split(strs[1], "+") cfg.QPS, err = strconv.ParseInt(strsb[0], 10, 64) if err != nil { - if err != nil { - log.Error("parse qps failed for case", zap.String("case", name), zap.String("config", strsb[0])) - } + log.Error("parse qps failed for case", zap.String("case", name), zap.String("config", strsb[0])) } // to get case Burst if len(strsb) > 1 { diff --git a/tools/pd-heartbeat-bench/main.go b/tools/pd-heartbeat-bench/main.go index ec5e2506e6b..77ae6354bff 100644 --- a/tools/pd-heartbeat-bench/main.go +++ b/tools/pd-heartbeat-bench/main.go @@ -475,7 +475,7 @@ func main() { } // New zap logger - err = logutil.SetupLogger(cfg.Log, &cfg.Logger, &cfg.LogProps) + err = logutil.SetupLogger(cfg.Log, &cfg.Logger, &cfg.LogProps, logutil.RedactInfoLogOFF) if err == nil { log.ReplaceGlobals(cfg.Logger, cfg.LogProps) } else { diff --git a/tools/pd-simulator/main.go b/tools/pd-simulator/main.go index 934cb7fd54f..63ba7f9134d 100644 --- a/tools/pd-simulator/main.go +++ b/tools/pd-simulator/main.go @@ -111,7 +111,7 @@ func run(simCase string, simConfig *sc.SimConfig) { // NewSingleServer creates a pd server for simulator. func NewSingleServer(ctx context.Context, simConfig *sc.SimConfig) (*server.Server, testutil.CleanupFunc) { - err := logutil.SetupLogger(simConfig.ServerConfig.Log, &simConfig.ServerConfig.Logger, &simConfig.ServerConfig.LogProps) + err := logutil.SetupLogger(simConfig.ServerConfig.Log, &simConfig.ServerConfig.Logger, &simConfig.ServerConfig.LogProps, simConfig.ServerConfig.Security.RedactInfoLog) if err == nil { log.ReplaceGlobals(simConfig.ServerConfig.Logger, simConfig.ServerConfig.LogProps) } else {