Skip to content

Commit d512c70

Browse files
helsaawyhelsaawy
authored
Scrubbing env vars from logs (#1315)
Added code to remove environment variables from code path. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
1 parent f50f975 commit d512c70

File tree

11 files changed

+1085
-624
lines changed

11 files changed

+1085
-624
lines changed

‎cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go‎

Lines changed: 330 additions & 304 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cmd/containerd-shim-runhcs-v1/options/runhcs.proto‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ message Options {
105105
// no_inherit_host_timezone specifies to skip inheriting the hosts time zone for WCOW UVMs and instead default to
106106
// UTC.
107107
bool no_inherit_host_timezone = 19;
108+
109+
// scrub_logs enables removing environment variables and other protentially sensitive information from logs
110+
bool scrub_logs = 20;
108111
}
109112

110113
// ProcessDetails contains additional information about a process. This is the additional

‎cmd/containerd-shim-runhcs-v1/serve.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ import (
1212
"unsafe"
1313

1414
"github.com/Microsoft/go-winio"
15-
runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
16-
"github.com/Microsoft/hcsshim/internal/extendedtask"
17-
"github.com/Microsoft/hcsshim/internal/shimdiag"
18-
"github.com/Microsoft/hcsshim/pkg/octtrpc"
1915
"github.com/containerd/containerd/log"
2016
"github.com/containerd/containerd/runtime/v2/task"
2117
"github.com/containerd/ttrpc"
@@ -26,6 +22,12 @@ import (
2622
"github.com/sirupsen/logrus"
2723
"github.com/urfave/cli"
2824
"golang.org/x/sys/windows"
25+
26+
runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
27+
"github.com/Microsoft/hcsshim/internal/extendedtask"
28+
hcslog "github.com/Microsoft/hcsshim/internal/log"
29+
"github.com/Microsoft/hcsshim/internal/shimdiag"
30+
"github.com/Microsoft/hcsshim/pkg/octtrpc"
2931
)
3032

3133
var svc *service
@@ -158,6 +160,11 @@ var serveCommand = cli.Command{
158160

159161
os.Stdin.Close()
160162

163+
// enable scrubbing
164+
if shimOpts.ScrubLogs {
165+
hcslog.SetScrubbing(true)
166+
}
167+
161168
// Force the cli.ErrWriter to be os.Stdout for this. We use stderr for
162169
// the panic.log attached via start.
163170
cli.ErrWriter = os.Stdout

‎internal/gcs/bridge.go‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import (
1717

1818
"github.com/sirupsen/logrus"
1919
"golang.org/x/sys/windows"
20+
21+
"github.com/Microsoft/hcsshim/internal/log"
2022
)
2123

2224
const (
@@ -365,6 +367,7 @@ func (brdg *bridge) recvLoop() error {
365367
func (brdg *bridge) sendLoop() {
366368
var buf bytes.Buffer
367369
enc := json.NewEncoder(&buf)
370+
enc.SetEscapeHTML(false)
368371
for {
369372
select {
370373
case <-brdg.waitCh:
@@ -392,11 +395,26 @@ func (brdg *bridge) writeMessage(buf *bytes.Buffer, enc *json.Encoder, typ msgTy
392395
}
393396
// Update the message header with the size.
394397
binary.LittleEndian.PutUint32(buf.Bytes()[hdrOffSize:], uint32(buf.Len()))
398+
399+
if brdg.log.Logger.GetLevel() >= logrus.DebugLevel {
400+
b := buf.Bytes()[hdrSize:]
401+
switch typ {
402+
// container environment vars are in rpCreate for linux; rpcExecuteProcess for windows
403+
case msgType(rpcCreate) | msgTypeRequest:
404+
b, err = log.ScrubBridgeCreate(b)
405+
case msgType(rpcExecuteProcess) | msgTypeRequest:
406+
b, err = log.ScrubBridgeExecProcess(b)
407+
}
408+
if err != nil {
409+
brdg.log.WithError(err).Warning("could not scrub bridge payload")
410+
}
411+
brdg.log.WithFields(logrus.Fields{
412+
"payload": string(b),
413+
"type": typ,
414+
"message-id": id}).Debug("bridge send")
415+
}
416+
395417
// Write the message.
396-
brdg.log.WithFields(logrus.Fields{
397-
"payload": string(buf.Bytes()[hdrSize:]),
398-
"type": typ,
399-
"message-id": id}).Debug("bridge send")
400418
_, err = buf.WriteTo(brdg.conn)
401419
if err != nil {
402420
return fmt.Errorf("bridge write: %s", err)

‎internal/log/scrub.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package log
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"errors"
7+
"sync/atomic"
8+
9+
hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
10+
)
11+
12+
// This package scrubs objects of potentially sensitive information to pass to logging
13+
14+
type genMap = map[string]interface{}
15+
type scrubberFunc func(genMap) error
16+
17+
const ScrubbedReplacement = "<scrubbed>"
18+
19+
var (
20+
ErrUnknownType = errors.New("encoded object is of unknown type")
21+
22+
// case sensitive keywords, so "env" is not a substring on "Environment"
23+
_scrubKeywords = [][]byte{[]byte("env"), []byte("Environment")}
24+
25+
_scrub int32
26+
)
27+
28+
// SetScrubbing enables scrubbing
29+
func SetScrubbing(enable bool) {
30+
v := int32(0) // cant convert from bool to int32 directly
31+
if enable {
32+
v = 1
33+
}
34+
atomic.StoreInt32(&_scrub, v)
35+
}
36+
37+
// IsScrubbingEnabled checks if scrubbing is enabled
38+
func IsScrubbingEnabled() bool {
39+
v := atomic.LoadInt32(&_scrub)
40+
return v != 0
41+
}
42+
43+
// ScrubProcessParameters scrubs HCS Create Process requests with config parameters of
44+
// type internal/hcs/schema2.ScrubProcessParameters (aka hcsshema.ScrubProcessParameters)
45+
func ScrubProcessParameters(s string) (string, error) {
46+
// todo: deal with v1 ProcessConfig
47+
b := []byte(s)
48+
if !IsScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
49+
return s, nil
50+
}
51+
52+
pp := hcsschema.ProcessParameters{}
53+
if err := json.Unmarshal(b, &pp); err != nil {
54+
return "", err
55+
}
56+
pp.Environment = map[string]string{ScrubbedReplacement: ScrubbedReplacement}
57+
58+
buf := bytes.NewBuffer(b[:0])
59+
if err := encode(buf, pp); err != nil {
60+
return "", err
61+
}
62+
return buf.String(), nil
63+
}
64+
65+
// ScrubBridgeCreate scrubs requests sent over the bridge of type
66+
// internal/gcs/protocol.containerCreate wrapping an internal/hcsoci.linuxHostedSystem
67+
func ScrubBridgeCreate(b []byte) ([]byte, error) {
68+
return scrubBytes(b, scrubLinuxHostedSystem)
69+
}
70+
71+
func scrubLinuxHostedSystem(m genMap) error {
72+
if !isRequestBase(m) {
73+
return ErrUnknownType
74+
}
75+
if m, ok := index(m, "ContainerConfig"); ok {
76+
if m, ok := index(m, "OciSpecification"); ok {
77+
if m, ok := index(m, "process"); ok {
78+
if _, ok := m["env"]; ok {
79+
m["env"] = []string{ScrubbedReplacement}
80+
return nil
81+
}
82+
}
83+
}
84+
}
85+
return ErrUnknownType
86+
}
87+
88+
// ScrubBridgeExecProcess scrubs requests sent over the bridge of type
89+
// internal/gcs/protocol.containerExecuteProcess
90+
func ScrubBridgeExecProcess(b []byte) ([]byte, error) {
91+
return scrubBytes(b, scrubExecuteProcess)
92+
}
93+
94+
func scrubExecuteProcess(m genMap) error {
95+
if !isRequestBase(m) {
96+
return ErrUnknownType
97+
}
98+
if m, ok := index(m, "Settings"); ok {
99+
if ss, ok := m["ProcessParameters"]; ok {
100+
// ProcessParameters is a json encoded struct passed as a regular sting field
101+
s, ok := ss.(string)
102+
if !ok {
103+
return ErrUnknownType
104+
}
105+
106+
s, err := ScrubProcessParameters(s)
107+
if err != nil {
108+
return err
109+
}
110+
111+
m["ProcessParameters"] = s
112+
return nil
113+
}
114+
}
115+
return ErrUnknownType
116+
}
117+
118+
func scrubBytes(b []byte, scrub scrubberFunc) ([]byte, error) {
119+
if !IsScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
120+
return b, nil
121+
}
122+
123+
m := make(genMap)
124+
if err := json.Unmarshal(b, &m); err != nil {
125+
return nil, err
126+
}
127+
128+
// could use regexp, but if the env strings contain braces, the regexp fails
129+
// parsing into individual structs would require access to private structs
130+
if err := scrub(m); err != nil {
131+
return nil, err
132+
}
133+
134+
buf := &bytes.Buffer{}
135+
if err := encode(buf, m); err != nil {
136+
return nil, err
137+
}
138+
return buf.Bytes(), nil
139+
}
140+
141+
func encode(buf *bytes.Buffer, v interface{}) error {
142+
enc := json.NewEncoder(buf)
143+
enc.SetEscapeHTML(false)
144+
if err := enc.Encode(v); err != nil {
145+
return err
146+
}
147+
return nil
148+
}
149+
150+
func isRequestBase(m genMap) bool {
151+
// neither of these are (currently) `omitempty`
152+
_, a := m["ActivityId"]
153+
_, c := m["ContainerId"]
154+
return a && c
155+
}
156+
157+
// combination `m, ok := m[s]` and `m, ok := m.(genMap)`
158+
func index(m genMap, s string) (genMap, bool) {
159+
if m, ok := m[s]; ok {
160+
mm, ok := m.(genMap)
161+
return mm, ok
162+
}
163+
164+
return m, false
165+
}
166+
167+
func hasKeywords(b []byte) bool {
168+
for _, bb := range _scrubKeywords {
169+
if bytes.Contains(b, bb) {
170+
return true
171+
}
172+
}
173+
return false
174+
}

‎internal/vmcompute/vmcompute.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ import (
55
"syscall"
66
"time"
77

8+
"go.opencensus.io/trace"
9+
810
"github.com/Microsoft/hcsshim/internal/interop"
911
"github.com/Microsoft/hcsshim/internal/log"
1012
"github.com/Microsoft/hcsshim/internal/logfields"
1113
"github.com/Microsoft/hcsshim/internal/oc"
1214
"github.com/Microsoft/hcsshim/internal/timeout"
13-
"go.opencensus.io/trace"
1415
)
1516

1617
//go:generate go run ../../mksyscall_windows.go -output zsyscall_windows.go vmcompute.go
@@ -389,7 +390,12 @@ func HcsCreateProcess(ctx gcontext.Context, computeSystem HcsSystem, processPara
389390
}
390391
oc.SetSpanStatus(span, hr)
391392
}()
392-
span.AddAttributes(trace.StringAttribute("processParameters", processParameters))
393+
if span.IsRecordingEvents() {
394+
// wont handle v1 process parameters
395+
if s, err := log.ScrubProcessParameters(processParameters); err == nil {
396+
span.AddAttributes(trace.StringAttribute("processParameters", s))
397+
}
398+
}
393399

394400
return processInformation, process, result, execute(ctx, timeout.SyscallWatcher, func() error {
395401
var resultp *uint16

0 commit comments

Comments
 (0)