Skip to content

Commit

Permalink
Scrubbing env vars from logs (#1315)
Browse files Browse the repository at this point in the history
Added code to remove environment variables from code path.

Signed-off-by: Hamza El-Saawy <[email protected]>
  • Loading branch information
helsaawy authored Mar 8, 2022
1 parent f50f975 commit d512c70
Show file tree
Hide file tree
Showing 11 changed files with 1,085 additions and 624 deletions.
634 changes: 330 additions & 304 deletions cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions cmd/containerd-shim-runhcs-v1/options/runhcs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ message Options {
// no_inherit_host_timezone specifies to skip inheriting the hosts time zone for WCOW UVMs and instead default to
// UTC.
bool no_inherit_host_timezone = 19;

// scrub_logs enables removing environment variables and other protentially sensitive information from logs
bool scrub_logs = 20;
}

// ProcessDetails contains additional information about a process. This is the additional
Expand Down
15 changes: 11 additions & 4 deletions cmd/containerd-shim-runhcs-v1/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ import (
"unsafe"

"github.com/Microsoft/go-winio"
runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/internal/extendedtask"
"github.com/Microsoft/hcsshim/internal/shimdiag"
"github.com/Microsoft/hcsshim/pkg/octtrpc"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/runtime/v2/task"
"github.com/containerd/ttrpc"
Expand All @@ -26,6 +22,12 @@ import (
"github.com/sirupsen/logrus"
"github.com/urfave/cli"
"golang.org/x/sys/windows"

runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/internal/extendedtask"
hcslog "github.com/Microsoft/hcsshim/internal/log"
"github.com/Microsoft/hcsshim/internal/shimdiag"
"github.com/Microsoft/hcsshim/pkg/octtrpc"
)

var svc *service
Expand Down Expand Up @@ -158,6 +160,11 @@ var serveCommand = cli.Command{

os.Stdin.Close()

// enable scrubbing
if shimOpts.ScrubLogs {
hcslog.SetScrubbing(true)
}

// Force the cli.ErrWriter to be os.Stdout for this. We use stderr for
// the panic.log attached via start.
cli.ErrWriter = os.Stdout
Expand Down
26 changes: 22 additions & 4 deletions internal/gcs/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (

"github.com/sirupsen/logrus"
"golang.org/x/sys/windows"

"github.com/Microsoft/hcsshim/internal/log"
)

const (
Expand Down Expand Up @@ -365,6 +367,7 @@ func (brdg *bridge) recvLoop() error {
func (brdg *bridge) sendLoop() {
var buf bytes.Buffer
enc := json.NewEncoder(&buf)
enc.SetEscapeHTML(false)
for {
select {
case <-brdg.waitCh:
Expand Down Expand Up @@ -392,11 +395,26 @@ func (brdg *bridge) writeMessage(buf *bytes.Buffer, enc *json.Encoder, typ msgTy
}
// Update the message header with the size.
binary.LittleEndian.PutUint32(buf.Bytes()[hdrOffSize:], uint32(buf.Len()))

if brdg.log.Logger.GetLevel() >= logrus.DebugLevel {
b := buf.Bytes()[hdrSize:]
switch typ {
// container environment vars are in rpCreate for linux; rpcExecuteProcess for windows
case msgType(rpcCreate) | msgTypeRequest:
b, err = log.ScrubBridgeCreate(b)
case msgType(rpcExecuteProcess) | msgTypeRequest:
b, err = log.ScrubBridgeExecProcess(b)
}
if err != nil {
brdg.log.WithError(err).Warning("could not scrub bridge payload")
}
brdg.log.WithFields(logrus.Fields{
"payload": string(b),
"type": typ,
"message-id": id}).Debug("bridge send")
}

// Write the message.
brdg.log.WithFields(logrus.Fields{
"payload": string(buf.Bytes()[hdrSize:]),
"type": typ,
"message-id": id}).Debug("bridge send")
_, err = buf.WriteTo(brdg.conn)
if err != nil {
return fmt.Errorf("bridge write: %s", err)
Expand Down
174 changes: 174 additions & 0 deletions internal/log/scrub.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
package log

import (
"bytes"
"encoding/json"
"errors"
"sync/atomic"

hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
)

// This package scrubs objects of potentially sensitive information to pass to logging

type genMap = map[string]interface{}
type scrubberFunc func(genMap) error

const ScrubbedReplacement = "<scrubbed>"

var (
ErrUnknownType = errors.New("encoded object is of unknown type")

// case sensitive keywords, so "env" is not a substring on "Environment"
_scrubKeywords = [][]byte{[]byte("env"), []byte("Environment")}

_scrub int32
)

// SetScrubbing enables scrubbing
func SetScrubbing(enable bool) {
v := int32(0) // cant convert from bool to int32 directly
if enable {
v = 1
}
atomic.StoreInt32(&_scrub, v)
}

// IsScrubbingEnabled checks if scrubbing is enabled
func IsScrubbingEnabled() bool {
v := atomic.LoadInt32(&_scrub)
return v != 0
}

// ScrubProcessParameters scrubs HCS Create Process requests with config parameters of
// type internal/hcs/schema2.ScrubProcessParameters (aka hcsshema.ScrubProcessParameters)
func ScrubProcessParameters(s string) (string, error) {
// todo: deal with v1 ProcessConfig
b := []byte(s)
if !IsScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
return s, nil
}

pp := hcsschema.ProcessParameters{}
if err := json.Unmarshal(b, &pp); err != nil {
return "", err
}
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
}

// 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)
}

func scrubLinuxHostedSystem(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
}
}
}
}
return ErrUnknownType
}

// ScrubBridgeExecProcess scrubs requests sent over the bridge of type
// internal/gcs/protocol.containerExecuteProcess
func ScrubBridgeExecProcess(b []byte) ([]byte, error) {
return scrubBytes(b, scrubExecuteProcess)
}

func scrubExecuteProcess(m genMap) error {
if !isRequestBase(m) {
return ErrUnknownType
}
if m, ok := index(m, "Settings"); ok {
if ss, ok := m["ProcessParameters"]; ok {
// ProcessParameters is a json encoded struct passed as a regular sting field
s, ok := ss.(string)
if !ok {
return ErrUnknownType
}

s, err := ScrubProcessParameters(s)
if err != nil {
return err
}

m["ProcessParameters"] = s
return nil
}
}
return ErrUnknownType
}

func scrubBytes(b []byte, scrub scrubberFunc) ([]byte, error) {
if !IsScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
return b, nil
}

m := make(genMap)
if err := json.Unmarshal(b, &m); err != nil {
return nil, err
}

// could use regexp, but if the env strings contain braces, the regexp fails
// parsing into individual structs would require access to private structs
if err := scrub(m); err != nil {
return nil, err
}

buf := &bytes.Buffer{}
if err := encode(buf, m); err != nil {
return nil, err
}
return buf.Bytes(), nil
}

func encode(buf *bytes.Buffer, v interface{}) error {
enc := json.NewEncoder(buf)
enc.SetEscapeHTML(false)
if err := enc.Encode(v); err != nil {
return err
}
return nil
}

func isRequestBase(m genMap) bool {
// neither of these are (currently) `omitempty`
_, a := m["ActivityId"]
_, c := m["ContainerId"]
return a && c
}

// combination `m, ok := m[s]` and `m, ok := m.(genMap)`
func index(m genMap, s string) (genMap, bool) {
if m, ok := m[s]; ok {
mm, ok := m.(genMap)
return mm, ok
}

return m, false
}

func hasKeywords(b []byte) bool {
for _, bb := range _scrubKeywords {
if bytes.Contains(b, bb) {
return true
}
}
return false
}
10 changes: 8 additions & 2 deletions internal/vmcompute/vmcompute.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import (
"syscall"
"time"

"go.opencensus.io/trace"

"github.com/Microsoft/hcsshim/internal/interop"
"github.com/Microsoft/hcsshim/internal/log"
"github.com/Microsoft/hcsshim/internal/logfields"
"github.com/Microsoft/hcsshim/internal/oc"
"github.com/Microsoft/hcsshim/internal/timeout"
"go.opencensus.io/trace"
)

//go:generate go run ../../mksyscall_windows.go -output zsyscall_windows.go vmcompute.go
Expand Down Expand Up @@ -389,7 +390,12 @@ func HcsCreateProcess(ctx gcontext.Context, computeSystem HcsSystem, processPara
}
oc.SetSpanStatus(span, hr)
}()
span.AddAttributes(trace.StringAttribute("processParameters", processParameters))
if span.IsRecordingEvents() {
// wont handle v1 process parameters
if s, err := log.ScrubProcessParameters(processParameters); err == nil {
span.AddAttributes(trace.StringAttribute("processParameters", s))
}
}

return processInformation, process, result, execute(ctx, timeout.SyscallWatcher, func() error {
var resultp *uint16
Expand Down
Loading

0 comments on commit d512c70

Please sign in to comment.