diff --git a/cmd/containerd-shim-lcow-v2/main.go b/cmd/containerd-shim-lcow-v2/main.go index 7b00c10e26..809260c37b 100644 --- a/cmd/containerd-shim-lcow-v2/main.go +++ b/cmd/containerd-shim-lcow-v2/main.go @@ -91,8 +91,10 @@ func setLogConfiguration() error { logrus.SetLevel(lvl) } - if opts.ScrubLogs { - log.SetScrubbing(true) + // Scrubbing is enabled by default (via init() in internal/log/scrub.go). + // Only disable if the option is explicitly set to false. + if opts.ScrubLogs != nil && !*opts.ScrubLogs { + log.SetScrubbing(false) } } _ = os.Stdin.Close() diff --git a/cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go b/cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go index 0a985d5aee..62ba956ea8 100644 --- a/cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go +++ b/cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go @@ -187,8 +187,9 @@ type Options struct { // no_inherit_host_timezone specifies to skip inheriting the hosts time zone for WCOW UVMs and instead default to // UTC. NoInheritHostTimezone bool `protobuf:"varint,19,opt,name=no_inherit_host_timezone,json=noInheritHostTimezone,proto3" json:"no_inherit_host_timezone,omitempty"` - // scrub_logs enables removing environment variables and other potentially sensitive information from logs - ScrubLogs bool `protobuf:"varint,20,opt,name=scrub_logs,json=scrubLogs,proto3" json:"scrub_logs,omitempty"` + // scrub_logs controls removing environment variables and other potentially sensitive information from logs. + // If unset, scrubbing is enabled by default. Set explicitly to false to disable. + ScrubLogs *bool `protobuf:"varint,20,opt,name=scrub_logs,json=scrubLogs,proto3,oneof" json:"scrub_logs,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -357,8 +358,8 @@ func (x *Options) GetNoInheritHostTimezone() bool { } func (x *Options) GetScrubLogs() bool { - if x != nil { - return x.ScrubLogs + if x != nil && x.ScrubLogs != nil { + return *x.ScrubLogs } return false } @@ -477,7 +478,7 @@ var File_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_runh const file_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_runhcs_proto_rawDesc = "" + "\n" + - "Ogithub.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options/runhcs.proto\x12\x14containerd.runhcs.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xd9\t\n" + + "Ogithub.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options/runhcs.proto\x12\x14containerd.runhcs.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xed\t\n" + "\aOptions\x12\x14\n" + "\x05debug\x18\x01 \x01(\bR\x05debug\x12F\n" + "\n" + @@ -501,9 +502,9 @@ const file_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_ru "\tlog_level\x18\x10 \x01(\tR\blogLevel\x124\n" + "\x17io_retry_timeout_in_sec\x18\x11 \x01(\x05R\x13ioRetryTimeoutInSec\x12\x82\x01\n" + "\x1ddefault_container_annotations\x18\x12 \x03(\v2>.containerd.runhcs.v1.Options.DefaultContainerAnnotationsEntryR\x1bdefaultContainerAnnotations\x127\n" + - "\x18no_inherit_host_timezone\x18\x13 \x01(\bR\x15noInheritHostTimezone\x12\x1d\n" + + "\x18no_inherit_host_timezone\x18\x13 \x01(\bR\x15noInheritHostTimezone\x12\"\n" + "\n" + - "scrub_logs\x18\x14 \x01(\bR\tscrubLogs\x1aN\n" + + "scrub_logs\x18\x14 \x01(\bH\x00R\tscrubLogs\x88\x01\x01\x1aN\n" + " DefaultContainerAnnotationsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\")\n" + @@ -514,7 +515,8 @@ const file_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_ru "\x10SandboxIsolation\x12\v\n" + "\aPROCESS\x10\x00\x12\x0e\n" + "\n" + - "HYPERVISOR\x10\x01\"\xb6\x03\n" + + "HYPERVISOR\x10\x01B\r\n" + + "\v_scrub_logs\"\xb6\x03\n" + "\x0eProcessDetails\x12\x1d\n" + "\n" + "image_name\x18\x01 \x01(\tR\timageName\x129\n" + @@ -570,6 +572,7 @@ func file_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_run if File_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_runhcs_proto != nil { return } + file_github_com_Microsoft_hcsshim_cmd_containerd_shim_runhcs_v1_options_runhcs_proto_msgTypes[0].OneofWrappers = []any{} type x struct{} out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ diff --git a/cmd/containerd-shim-runhcs-v1/options/runhcs.proto b/cmd/containerd-shim-runhcs-v1/options/runhcs.proto index 8546c61507..b913f3c34f 100644 --- a/cmd/containerd-shim-runhcs-v1/options/runhcs.proto +++ b/cmd/containerd-shim-runhcs-v1/options/runhcs.proto @@ -105,8 +105,9 @@ message Options { // UTC. bool no_inherit_host_timezone = 19; - // scrub_logs enables removing environment variables and other potentially sensitive information from logs - bool scrub_logs = 20; + // scrub_logs controls removing environment variables and other potentially sensitive information from logs. + // If unset, scrubbing is enabled by default. Set explicitly to false to disable. + optional bool scrub_logs = 20; } // ProcessDetails contains additional information about a process. This is the additional diff --git a/cmd/containerd-shim-runhcs-v1/serve.go b/cmd/containerd-shim-runhcs-v1/serve.go index f2f8d5ce39..d7e236ac6e 100644 --- a/cmd/containerd-shim-runhcs-v1/serve.go +++ b/cmd/containerd-shim-runhcs-v1/serve.go @@ -161,9 +161,10 @@ var serveCommand = cli.Command{ os.Stdin.Close() - // enable scrubbing - if shimOpts.ScrubLogs { - hcslog.SetScrubbing(true) + // Scrubbing is enabled by default (via init() in internal/log/scrub.go). + // Only disable if the option is explicitly set to false. + if shimOpts.ScrubLogs != nil && !*shimOpts.ScrubLogs { + hcslog.SetScrubbing(false) } // Force the cli.ErrWriter to be os.Stdout for this. We use stderr for diff --git a/cmd/gcs/main.go b/cmd/gcs/main.go index 68ab28d499..ef726fa127 100644 --- a/cmd/gcs/main.go +++ b/cmd/gcs/main.go @@ -221,7 +221,7 @@ func main() { 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") + scrubLogs := flag.Bool("scrub-logs", true, "If true, scrub potentially sensitive information from logging") initialPolicyStance := flag.String("initial-policy-stance", "allow", "Stance: allow, deny.") diff --git a/internal/builder/vm/lcow/kernel_args.go b/internal/builder/vm/lcow/kernel_args.go index 21222761e1..6d464d0d9c 100644 --- a/internal/builder/vm/lcow/kernel_args.go +++ b/internal/builder/vm/lcow/kernel_args.go @@ -5,6 +5,7 @@ package lcow import ( "context" "fmt" + "strconv" "strings" runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" @@ -219,8 +220,9 @@ func buildGCSCommand( gcsParts = append(gcsParts, "-disable-time-sync") } - if opts != nil && opts.ScrubLogs { - gcsParts = append(gcsParts, "-scrub-logs") + // Scrubbing is enabled by default. Only pass the flag if explicitly set. + if opts != nil && opts.ScrubLogs != nil { + gcsParts = append(gcsParts, fmt.Sprintf("-scrub-logs=%s", strconv.FormatBool(*opts.ScrubLogs))) } if processDumpLocation != "" { diff --git a/internal/builder/vm/lcow/specs_test.go b/internal/builder/vm/lcow/specs_test.go index 1c168b32df..c3480b66fa 100644 --- a/internal/builder/vm/lcow/specs_test.go +++ b/internal/builder/vm/lcow/specs_test.go @@ -20,6 +20,7 @@ import ( vm "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2" "github.com/opencontainers/runtime-spec/specs-go" + "google.golang.org/protobuf/proto" ) type specTestCase struct { @@ -1638,11 +1639,11 @@ func TestBuildSandboxConfig_BootOptions(t *testing.T) { }, }, { - name: "scrub logs option", + name: "scrub logs option enabled", opts: &runhcsoptions.Options{ SandboxPlatform: "linux/amd64", BootFilesRootPath: vhdOnlyPath, - ScrubLogs: true, + ScrubLogs: proto.Bool(true), }, validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) { t.Helper() @@ -1651,6 +1652,34 @@ func TestBuildSandboxConfig_BootOptions(t *testing.T) { } }, }, + { + name: "scrub logs option disabled", + opts: &runhcsoptions.Options{ + SandboxPlatform: "linux/amd64", + BootFilesRootPath: vhdOnlyPath, + ScrubLogs: proto.Bool(false), + }, + validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) { + t.Helper() + if !strings.Contains(getKernelArgs(doc), "-scrub-logs=false") { + t.Error("expected -scrub-logs=false in kernel args") + } + }, + }, + { + name: "scrub logs option unset", + opts: &runhcsoptions.Options{ + SandboxPlatform: "linux/amd64", + BootFilesRootPath: vhdOnlyPath, + }, + validate: func(t *testing.T, doc *hcsschema.ComputeSystem, sandboxOpts *SandboxOptions) { + t.Helper() + args := getKernelArgs(doc) + if strings.Contains(args, "-scrub-logs") { + t.Error("did not expect -scrub-logs in kernel args when unset") + } + }, + }, } runTestCases(t, ctx, nil, tests) diff --git a/internal/hcsoci/create.go b/internal/hcsoci/create.go index b12d73274f..5288932fa1 100644 --- a/internal/hcsoci/create.go +++ b/internal/hcsoci/create.go @@ -128,10 +128,17 @@ func initializeCreateOptions(ctx context.Context, createOptions *CreateOptions) coi.actualSchemaVersion = schemaversion.DetermineSchemaVersion(coi.SchemaVersion) } - log.G(ctx).WithFields(logrus.Fields{ - "options": log.Format(ctx, createOptions), - "schema": log.Format(ctx, coi.actualSchemaVersion), - }).Debug("hcsshim::initializeCreateOptions") + // Log create options if debug logging is enabled + if logrus.IsLevelEnabled(logrus.DebugLevel) { + if b, err := log.ScrubCreateOptions([]byte(log.Format(ctx, createOptions))); err != nil { + log.G(ctx).WithError(err).Warning("could not scrub CreateOptions") + } else { + log.G(ctx).WithFields(logrus.Fields{ + "options": string(b), + "schema": log.Format(ctx, coi.actualSchemaVersion), + }).Debug("hcsshim::initializeCreateOptions") + } + } return coi, nil } diff --git a/internal/log/scrub.go b/internal/log/scrub.go index 83ee97f853..455bc5262d 100644 --- a/internal/log/scrub.go +++ b/internal/log/scrub.go @@ -29,7 +29,13 @@ var ( _scrub atomic.Bool ) -// SetScrubbing enables scrubbing +func init() { + // Scrubbing is enabled by default to prevent sensitive information + // (such as environment variables containing secrets) from leaking to logs. + _scrub.Store(true) +} + +// SetScrubbing enables or disables scrubbing of potentially sensitive information from logging. func SetScrubbing(enable bool) { _scrub.Store(enable) } // IsScrubbingEnabled checks if scrubbing is enabled @@ -174,6 +180,19 @@ func isRequestBase(m genMap) bool { return a && c } +// ScrubCreateOptions scrubs a JSON-encoded CreateOptions struct, +// removing sensitive fields (env vars, annotations) from the embedded OCI Spec. +func ScrubCreateOptions(b []byte) ([]byte, error) { + return scrubBytes(b, scrubCreateOptions) +} + +func scrubCreateOptions(m genMap) error { + if spec, ok := index(m, "Spec"); ok { + return scrubOCISpec(spec) + } + return nil +} + // combination `m, ok := m[s]` and `m, ok := m.(genMap)` func index(m genMap, s string) (genMap, bool) { if m, ok := m[s]; ok { diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 9ac7018665..d5eb2ee814 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -841,8 +841,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" + // Scrubbing is enabled by default in GCS. Only pass the flag when scrubbing + // has been explicitly disabled on the host to inform GCS to turn it off. + if !log.IsScrubbingEnabled() { + opts.ExecCommandLine += " -scrub-logs=false" } execCmdArgs += " " + opts.ExecCommandLine