From 797beb7de4209820c178532b6acb3fcffa82eab9 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Tue, 1 Mar 2022 00:37:54 -0500 Subject: [PATCH 1/2] Scrubbing annotations from logs Updated scrubbing for guest side, and added scrubbing for annotations Signed-off-by: Hamza El-Saawy --- cmd/gcs/main.go | 4 +++ internal/guest/bridge/bridge.go | 15 +++++++-- internal/log/scrub.go | 58 +++++++++++++++++++++++++-------- internal/uvm/create_lcow.go | 4 +++ 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/cmd/gcs/main.go b/cmd/gcs/main.go index cad6948cd1..0926ef4d4b 100644 --- a/cmd/gcs/main.go +++ b/cmd/gcs/main.go @@ -19,6 +19,7 @@ import ( "github.com/Microsoft/hcsshim/internal/guest/runtime/hcsv2" "github.com/Microsoft/hcsshim/internal/guest/runtime/runc" "github.com/Microsoft/hcsshim/internal/guest/transport" + "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" "github.com/cenkalti/backoff/v4" "github.com/containerd/cgroups" @@ -176,6 +177,7 @@ func main() { rootMemReserveBytes := flag.Uint64("root-mem-reserve-bytes", 75*1024*1024, "the amount of memory reserved for the orchestration, the rest will be assigned to containers") gcsMemLimitBytes := flag.Uint64("gcs-mem-limit-bytes", 50*1024*1024, "the maximum amount of memory the gcs can use") disableTimeSync := flag.Bool("disable-time-sync", false, "If true do not run chronyd time synchronization service inside the UVM") + scrubLogs := flag.Bool("scrub-logs", false, "If true, scrub potentially sensitive information from logging") flag.Usage = func() { fmt.Fprintf(os.Stderr, "\nUsage of %s:\n", os.Args[0]) @@ -225,6 +227,8 @@ func main() { logrus.SetLevel(level) + log.SetScrubbing(*scrubLogs) + baseLogPath := "/run/gcs/c" logrus.Info("GCS started") diff --git a/internal/guest/bridge/bridge.go b/internal/guest/bridge/bridge.go index 2d2e56c768..b9005c4bb1 100644 --- a/internal/guest/bridge/bridge.go +++ b/internal/guest/bridge/bridge.go @@ -301,8 +301,19 @@ func (b *Bridge) ListenAndServe(bridgeIn io.ReadCloser, bridgeOut io.WriteCloser trace.StringAttribute("activityID", base.ActivityID), trace.StringAttribute("cid", base.ContainerID)) - log.G(ctx).WithField("message", string(message)).Debug("request read message") - + etry := log.G(ctx) + if etry.Logger.GetLevel() >= logrus.DebugLevel { + s := string(message) + switch header.Type { + case prot.ComputeSystemCreateV1: + b, err := log.ScrubBridgeCreate(message) + s = string(b) + if err != nil { + etry.WithError(err).Warning("could not scrub bridge payload") + } + } + etry.WithField("message", s).Debug("request read message") + } requestChan <- &Request{ Context: ctx, Header: header, diff --git a/internal/log/scrub.go b/internal/log/scrub.go index f66913941f..901f40b86b 100644 --- a/internal/log/scrub.go +++ b/internal/log/scrub.go @@ -14,7 +14,7 @@ import ( type genMap = map[string]interface{} type scrubberFunc func(genMap) error -const ScrubbedReplacement = "" +const _scrubbedReplacement = "" var ( ErrUnknownType = errors.New("encoded object is of unknown type") @@ -53,32 +53,50 @@ func ScrubProcessParameters(s string) (string, error) { if err := json.Unmarshal(b, &pp); err != nil { return "", err } - pp.Environment = map[string]string{ScrubbedReplacement: ScrubbedReplacement} + pp.Environment = map[string]string{_scrubbedReplacement: _scrubbedReplacement} buf := bytes.NewBuffer(b[:0]) if err := encode(buf, pp); err != nil { return "", err } - return buf.String(), nil + return trimStringNewline(s), nil } // ScrubBridgeCreate scrubs requests sent over the bridge of type // internal/gcs/protocol.containerCreate wrapping an internal/hcsoci.linuxHostedSystem func ScrubBridgeCreate(b []byte) ([]byte, error) { - return scrubBytes(b, scrubLinuxHostedSystem) + return scrubBytes(b, scrubBridgeCreate) } -func scrubLinuxHostedSystem(m genMap) error { +func scrubBridgeCreate(m genMap) error { if !isRequestBase(m) { return ErrUnknownType } - if m, ok := index(m, "ContainerConfig"); ok { - if m, ok := index(m, "OciSpecification"); ok { - if m, ok := index(m, "process"); ok { - if _, ok := m["env"]; ok { - m["env"] = []string{ScrubbedReplacement} - return nil - } + if ss, ok := m["ContainerConfig"]; ok { + // ContainerConfig is a json encoded struct passed as a regular sting field + s, ok := ss.(string) + if !ok { + return ErrUnknownType + } + b, err := scrubBytes([]byte(s), scrubLinuxHostedSystem) + if err != nil { + return err + } + m["ContainerConfig"] = string(b) + return nil + } + return ErrUnknownType +} + +func scrubLinuxHostedSystem(m genMap) error { + if m, ok := index(m, "OciSpecification"); ok { + if _, ok := m["annotations"]; ok { + m["annotations"] = map[string]string{_scrubbedReplacement: _scrubbedReplacement} + } + if m, ok := index(m, "process"); ok { + if _, ok := m["env"]; ok { + m["env"] = []string{_scrubbedReplacement} + return nil } } } @@ -135,7 +153,13 @@ func scrubBytes(b []byte, scrub scrubberFunc) ([]byte, error) { if err := encode(buf, m); err != nil { return nil, err } - return buf.Bytes(), nil + + // remove newline encode appends + bb := buf.Bytes() + if len(bb) > 0 && bb[len(bb)-1] == '\n' { + bb = bb[:len(bb)-1] + } + return bb, nil } func encode(buf *bytes.Buffer, v interface{}) error { @@ -172,3 +196,11 @@ func hasKeywords(b []byte) bool { } return false } + +func trimStringNewline(s string) string { + l := len(s) + if l > 0 && s[l-1] == '\n' { + return s[:l-1] + } + return s +} diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 1211f78ceb..616c2872cc 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -657,6 +657,10 @@ func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcs opts.ExecCommandLine = fmt.Sprintf("%s --disable-time-sync", opts.ExecCommandLine) } + if log.IsScrubbingEnabled() { + opts.ExecCommandLine = fmt.Sprintf("%s --scrub-logs", opts.ExecCommandLine) + } + initArgs += " " + opts.ExecCommandLine if opts.ProcessDumpLocation != "" { From 20782ee5574d0f385a768c0cd3ed34d99f3b145f Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Fri, 11 Mar 2022 18:01:17 -0500 Subject: [PATCH 2/2] PR: builtin functions and vendor Signed-off-by: Hamza El-Saawy --- internal/guest/bridge/bridge.go | 8 ++-- internal/log/scrub.go | 20 ++------ internal/uvm/create_lcow.go | 2 +- .../Microsoft/hcsshim/internal/log/scrub.go | 46 +++++++++++++------ .../hcsshim/internal/uvm/create_lcow.go | 4 ++ 5 files changed, 46 insertions(+), 34 deletions(-) diff --git a/internal/guest/bridge/bridge.go b/internal/guest/bridge/bridge.go index b9005c4bb1..6b371950b0 100644 --- a/internal/guest/bridge/bridge.go +++ b/internal/guest/bridge/bridge.go @@ -301,18 +301,18 @@ func (b *Bridge) ListenAndServe(bridgeIn io.ReadCloser, bridgeOut io.WriteCloser trace.StringAttribute("activityID", base.ActivityID), trace.StringAttribute("cid", base.ContainerID)) - etry := log.G(ctx) - if etry.Logger.GetLevel() >= logrus.DebugLevel { + entry := log.G(ctx) + if entry.Logger.GetLevel() >= logrus.DebugLevel { s := string(message) switch header.Type { case prot.ComputeSystemCreateV1: b, err := log.ScrubBridgeCreate(message) s = string(b) if err != nil { - etry.WithError(err).Warning("could not scrub bridge payload") + entry.WithError(err).Warning("could not scrub bridge payload") } } - etry.WithField("message", s).Debug("request read message") + entry.WithField("message", s).Debug("request read message") } requestChan <- &Request{ Context: ctx, diff --git a/internal/log/scrub.go b/internal/log/scrub.go index 901f40b86b..b906a16bb4 100644 --- a/internal/log/scrub.go +++ b/internal/log/scrub.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "strings" "sync/atomic" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" @@ -59,7 +60,7 @@ func ScrubProcessParameters(s string) (string, error) { if err := encode(buf, pp); err != nil { return "", err } - return trimStringNewline(s), nil + return strings.TrimSpace(s), nil } // ScrubBridgeCreate scrubs requests sent over the bridge of type @@ -73,7 +74,7 @@ func scrubBridgeCreate(m genMap) error { return ErrUnknownType } if ss, ok := m["ContainerConfig"]; ok { - // ContainerConfig is a json encoded struct passed as a regular sting field + // ContainerConfig is a json encoded struct passed as a regular string field s, ok := ss.(string) if !ok { return ErrUnknownType @@ -154,12 +155,7 @@ func scrubBytes(b []byte, scrub scrubberFunc) ([]byte, error) { return nil, err } - // remove newline encode appends - bb := buf.Bytes() - if len(bb) > 0 && bb[len(bb)-1] == '\n' { - bb = bb[:len(bb)-1] - } - return bb, nil + return bytes.TrimSpace(buf.Bytes()), nil } func encode(buf *bytes.Buffer, v interface{}) error { @@ -196,11 +192,3 @@ func hasKeywords(b []byte) bool { } return false } - -func trimStringNewline(s string) string { - l := len(s) - if l > 0 && s[l-1] == '\n' { - return s[:l-1] - } - return s -} diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 616c2872cc..e98b0f0690 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -658,7 +658,7 @@ func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcs } if log.IsScrubbingEnabled() { - opts.ExecCommandLine = fmt.Sprintf("%s --scrub-logs", opts.ExecCommandLine) + opts.ExecCommandLine += " --scrub-logs" } initArgs += " " + opts.ExecCommandLine diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/log/scrub.go b/test/vendor/github.com/Microsoft/hcsshim/internal/log/scrub.go index f66913941f..b906a16bb4 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/log/scrub.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/log/scrub.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "strings" "sync/atomic" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" @@ -14,7 +15,7 @@ import ( type genMap = map[string]interface{} type scrubberFunc func(genMap) error -const ScrubbedReplacement = "" +const _scrubbedReplacement = "" var ( ErrUnknownType = errors.New("encoded object is of unknown type") @@ -53,32 +54,50 @@ func ScrubProcessParameters(s string) (string, error) { if err := json.Unmarshal(b, &pp); err != nil { return "", err } - pp.Environment = map[string]string{ScrubbedReplacement: ScrubbedReplacement} + pp.Environment = map[string]string{_scrubbedReplacement: _scrubbedReplacement} buf := bytes.NewBuffer(b[:0]) if err := encode(buf, pp); err != nil { return "", err } - return buf.String(), nil + return strings.TrimSpace(s), nil } // ScrubBridgeCreate scrubs requests sent over the bridge of type // internal/gcs/protocol.containerCreate wrapping an internal/hcsoci.linuxHostedSystem func ScrubBridgeCreate(b []byte) ([]byte, error) { - return scrubBytes(b, scrubLinuxHostedSystem) + return scrubBytes(b, scrubBridgeCreate) } -func scrubLinuxHostedSystem(m genMap) error { +func scrubBridgeCreate(m genMap) error { if !isRequestBase(m) { return ErrUnknownType } - if m, ok := index(m, "ContainerConfig"); ok { - if m, ok := index(m, "OciSpecification"); ok { - if m, ok := index(m, "process"); ok { - if _, ok := m["env"]; ok { - m["env"] = []string{ScrubbedReplacement} - return nil - } + if ss, ok := m["ContainerConfig"]; ok { + // ContainerConfig is a json encoded struct passed as a regular string field + s, ok := ss.(string) + if !ok { + return ErrUnknownType + } + b, err := scrubBytes([]byte(s), scrubLinuxHostedSystem) + if err != nil { + return err + } + m["ContainerConfig"] = string(b) + return nil + } + return ErrUnknownType +} + +func scrubLinuxHostedSystem(m genMap) error { + if m, ok := index(m, "OciSpecification"); ok { + if _, ok := m["annotations"]; ok { + m["annotations"] = map[string]string{_scrubbedReplacement: _scrubbedReplacement} + } + if m, ok := index(m, "process"); ok { + if _, ok := m["env"]; ok { + m["env"] = []string{_scrubbedReplacement} + return nil } } } @@ -135,7 +154,8 @@ func scrubBytes(b []byte, scrub scrubberFunc) ([]byte, error) { if err := encode(buf, m); err != nil { return nil, err } - return buf.Bytes(), nil + + return bytes.TrimSpace(buf.Bytes()), nil } func encode(buf *bytes.Buffer, v interface{}) error { diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go index 1211f78ceb..e98b0f0690 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/create_lcow.go @@ -657,6 +657,10 @@ func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcs opts.ExecCommandLine = fmt.Sprintf("%s --disable-time-sync", opts.ExecCommandLine) } + if log.IsScrubbingEnabled() { + opts.ExecCommandLine += " --scrub-logs" + } + initArgs += " " + opts.ExecCommandLine if opts.ProcessDumpLocation != "" {