diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index f9838d90796..d3260f58e6f 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -446,7 +446,8 @@ const ( // owner: @aravindhp @LorbusChris // kep: http://kep.k8s.io/2271 // - // Enables querying logs of node services using the /logs endpoint + // Enables querying logs of node services using the /logs endpoint. Enabling this feature has security implications. + // The recommendation is to enable it on a need basis for debugging purposes and disabling otherwise. NodeLogQuery featuregate.Feature = "NodeLogQuery" // owner: @iholder101 @kannon92 diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index dbffb081c2c..5070323448c 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -64586,7 +64586,7 @@ func schema_k8sio_kubelet_config_v1beta1_KubeletConfiguration(ref common.Referen }, "enableSystemLogQuery": { SchemaProps: spec.SchemaProps{ - Description: "enableSystemLogQuery enables the node log query feature on the /logs endpoint. EnableSystemLogHandler has to be enabled in addition for this feature to work. Default: false", + Description: "enableSystemLogQuery enables the node log query feature on the /logs endpoint. EnableSystemLogHandler has to be enabled in addition for this feature to work. Enabling this feature has security implications. The recommendation is to enable it on a need basis for debugging purposes and disabling otherwise. Default: false", Type: []string{"boolean"}, Format: "", }, diff --git a/pkg/kubelet/apis/config/types.go b/pkg/kubelet/apis/config/types.go index 94ed741d05b..a7024cc993c 100644 --- a/pkg/kubelet/apis/config/types.go +++ b/pkg/kubelet/apis/config/types.go @@ -413,6 +413,8 @@ type KubeletConfiguration struct { EnableSystemLogHandler bool // EnableSystemLogQuery enables the node log query feature on the /logs endpoint. // EnableSystemLogHandler has to be enabled in addition for this feature to work. + // Enabling this feature has security implications. The recommendation is to enable it on a need basis for debugging + // purposes and disabling otherwise. // +featureGate=NodeLogQuery // +optional EnableSystemLogQuery bool diff --git a/pkg/kubelet/kubelet_server_journal.go b/pkg/kubelet/kubelet_server_journal.go index 0159910803e..27c3421b883 100644 --- a/pkg/kubelet/kubelet_server_journal.go +++ b/pkg/kubelet/kubelet_server_journal.go @@ -316,7 +316,7 @@ func (n *nodeLogQuery) splitNativeVsFileLoggers(ctx context.Context) ([]string, // copyServiceLogs invokes journalctl or Get-WinEvent with the provided args. Note that // services are explicitly passed here to account for the heuristics. func (n *nodeLogQuery) copyServiceLogs(ctx context.Context, w io.Writer, services []string, previousBoot int) { - cmdStr, args, err := getLoggingCmd(n, services) + cmdStr, args, cmdEnv, err := getLoggingCmd(n, services) if err != nil { fmt.Fprintf(w, "\nfailed to get logging cmd: %v\n", err) return @@ -324,6 +324,7 @@ func (n *nodeLogQuery) copyServiceLogs(ctx context.Context, w io.Writer, service cmd := exec.CommandContext(ctx, cmdStr, args...) cmd.Stdout = w cmd.Stderr = w + cmd.Env = append(os.Environ(), cmdEnv...) if err := cmd.Run(); err != nil { if _, ok := err.(*exec.ExitError); ok { diff --git a/pkg/kubelet/kubelet_server_journal_linux.go b/pkg/kubelet/kubelet_server_journal_linux.go index 0265d08fccb..bf550e6b946 100644 --- a/pkg/kubelet/kubelet_server_journal_linux.go +++ b/pkg/kubelet/kubelet_server_journal_linux.go @@ -31,9 +31,13 @@ import ( ) // getLoggingCmd returns the journalctl cmd and arguments for the given nodeLogQuery and boot. Note that -// services are explicitly passed here to account for the heuristics -func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error) { - args := []string{ +// services are explicitly passed here to account for the heuristics. +// The return values are: +// - cmd: the command to be executed +// - args: arguments to the command +// - cmdEnv: environment variables when the command will be executed +func getLoggingCmd(n *nodeLogQuery, services []string) (cmd string, args []string, cmdEnv []string, err error) { + args = []string{ "--utc", "--no-pager", "--output=short-precise", @@ -60,7 +64,7 @@ func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error) args = append(args, "--boot", fmt.Sprintf("%d", *n.Boot)) } - return "journalctl", args, nil + return "journalctl", args, nil, nil } // checkForNativeLogger checks journalctl output for a service diff --git a/pkg/kubelet/kubelet_server_journal_others.go b/pkg/kubelet/kubelet_server_journal_others.go index 562b0414409..33f1205431a 100644 --- a/pkg/kubelet/kubelet_server_journal_others.go +++ b/pkg/kubelet/kubelet_server_journal_others.go @@ -24,8 +24,8 @@ import ( ) // getLoggingCmd on unsupported operating systems returns the echo command and a warning message (as strings) -func getLoggingCmd(_ *nodeLogQuery, _ []string) (string, []string, error) { - return "", []string{}, errors.New("Operating System Not Supported") +func getLoggingCmd(_ *nodeLogQuery, _ []string) (cmd string, args []string, cmdEnv []string, err error) { + return "", args, cmdEnv, errors.New("Operating System Not Supported") } // checkForNativeLogger on unsupported operating systems returns false diff --git a/pkg/kubelet/kubelet_server_journal_test.go b/pkg/kubelet/kubelet_server_journal_test.go index 7e8c13c1f84..bf42f685f45 100644 --- a/pkg/kubelet/kubelet_server_journal_test.go +++ b/pkg/kubelet/kubelet_server_journal_test.go @@ -35,31 +35,62 @@ import ( ) func Test_getLoggingCmd(t *testing.T) { + var emptyCmdEnv []string tests := []struct { - name string - args nodeLogQuery - wantLinux []string - wantWindows []string - wantOtherOS []string + name string + args nodeLogQuery + services []string + wantLinux []string + wantWindows []string + wantLinuxCmdEnv []string + wantWindowsCmdEnv []string }{ { - args: nodeLogQuery{}, - wantLinux: []string{"--utc", "--no-pager", "--output=short-precise"}, - wantWindows: []string{"-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", "Get-WinEvent -FilterHashtable @{LogName='Application'} | Sort-Object TimeCreated | Format-Table -AutoSize -Wrap"}, + name: "basic", + args: nodeLogQuery{}, + services: []string{}, + wantLinux: []string{"--utc", "--no-pager", "--output=short-precise"}, + wantLinuxCmdEnv: emptyCmdEnv, + wantWindows: []string{"-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", "Get-WinEvent -FilterHashtable @{LogName='Application'} | Sort-Object TimeCreated | Format-Table -AutoSize -Wrap"}, + wantWindowsCmdEnv: emptyCmdEnv, + }, + { + name: "two providers", + args: nodeLogQuery{}, + services: []string{"p1", "p2"}, + wantLinux: []string{"--utc", "--no-pager", "--output=short-precise", "--unit=p1", "--unit=p2"}, + wantLinuxCmdEnv: emptyCmdEnv, + wantWindows: []string{"-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", "Get-WinEvent -FilterHashtable @{LogName='Application'; ProviderName=$Env:kubelet_provider0,$Env:kubelet_provider1} | Sort-Object TimeCreated | Format-Table -AutoSize -Wrap"}, + wantWindowsCmdEnv: []string{"kubelet_provider0=p1", "kubelet_provider1=p2"}, + }, + { + name: "empty provider", + args: nodeLogQuery{}, + services: []string{"p1", "", "p2"}, + wantLinux: []string{"--utc", "--no-pager", "--output=short-precise", "--unit=p1", "--unit=p2"}, + wantLinuxCmdEnv: emptyCmdEnv, + wantWindows: []string{"-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", "Get-WinEvent -FilterHashtable @{LogName='Application'; ProviderName=$Env:kubelet_provider0,$Env:kubelet_provider2} | Sort-Object TimeCreated | Format-Table -AutoSize -Wrap"}, + wantWindowsCmdEnv: []string{"kubelet_provider0=p1", "kubelet_provider2=p2"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, got, err := getLoggingCmd(&tt.args, []string{}) + _, got, gotCmdEnv, err := getLoggingCmd(&tt.args, tt.services) switch os := runtime.GOOS; os { case "linux": if !reflect.DeepEqual(got, tt.wantLinux) { t.Errorf("getLoggingCmd() = %v, want %v", got, tt.wantLinux) } + if !reflect.DeepEqual(gotCmdEnv, tt.wantLinuxCmdEnv) { + t.Errorf("gotCmdEnv %v, wantLinuxCmdEnv %v", gotCmdEnv, tt.wantLinuxCmdEnv) + } case "windows": if !reflect.DeepEqual(got, tt.wantWindows) { t.Errorf("getLoggingCmd() = %v, want %v", got, tt.wantWindows) } + if !reflect.DeepEqual(gotCmdEnv, tt.wantWindowsCmdEnv) { + t.Errorf("gotCmdEnv %v, wantWindowsCmdEnv %v", gotCmdEnv, tt.wantWindowsCmdEnv) + } default: if err == nil { t.Errorf("getLoggingCmd() = %v, want err", got) diff --git a/pkg/kubelet/kubelet_server_journal_windows.go b/pkg/kubelet/kubelet_server_journal_windows.go index a805cfc5453..ffe2df1772d 100644 --- a/pkg/kubelet/kubelet_server_journal_windows.go +++ b/pkg/kubelet/kubelet_server_journal_windows.go @@ -27,43 +27,107 @@ import ( const powershellExe = "PowerShell.exe" -// getLoggingCmd returns the powershell cmd and arguments for the given nodeLogQuery and boot -func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error) { - args := []string{ +// getLoggingCmd returns the powershell cmd, arguments, and environment variables for the given nodeLogQuery and boot. +// All string inputs are environment variables to stop subcommands expressions from being executed. +// The return values are: +// - cmd: the command to be executed +// - args: arguments to the command +// - cmdEnv: environment variables when the command will be executed +func getLoggingCmd(n *nodeLogQuery, services []string) (cmd string, args []string, cmdEnv []string, err error) { + cmdEnv = getLoggingCmdEnv(n, services) + + var includeSinceTime, includeUntilTime, includeTailLines, includePattern bool + if n.SinceTime != nil { + includeSinceTime = true + } + if n.UntilTime != nil { + includeUntilTime = true + } + if n.TailLines != nil { + includeTailLines = true + } + if len(n.Pattern) > 0 { + includePattern = true + } + + var includeServices []bool + for _, service := range services { + includeServices = append(includeServices, len(service) > 0) + } + + args = getLoggingCmdArgs(includeSinceTime, includeUntilTime, includeTailLines, includePattern, includeServices) + + return powershellExe, args, cmdEnv, nil +} + +// getLoggingCmdArgs returns arguments that need to be passed to powershellExe +func getLoggingCmdArgs(includeSinceTime, includeUntilTime, includeTailLines, includePattern bool, services []bool) (args []string) { + args = []string{ "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", } - psCmd := "Get-WinEvent -FilterHashtable @{LogName='Application'" - if n.SinceTime != nil { - psCmd += fmt.Sprintf("; StartTime='%s'", n.SinceTime.Format(dateLayout)) + psCmd := `Get-WinEvent -FilterHashtable @{LogName='Application'` + + if includeSinceTime { + psCmd += fmt.Sprintf(`; StartTime="$Env:kubelet_sinceTime"`) } - if n.UntilTime != nil { - psCmd += fmt.Sprintf("; EndTime='%s'", n.UntilTime.Format(dateLayout)) + if includeUntilTime { + psCmd += fmt.Sprintf(`; EndTime="$Env:kubelet_untilTime"`) } + var providers []string - for _, service := range services { - if len(service) > 0 { - providers = append(providers, "'"+service+"'") + for i := range services { + if services[i] { + providers = append(providers, fmt.Sprintf("$Env:kubelet_provider%d", i)) } } + if len(providers) > 0 { psCmd += fmt.Sprintf("; ProviderName=%s", strings.Join(providers, ",")) } - psCmd += "}" - if n.TailLines != nil { - psCmd += fmt.Sprintf(" -MaxEvents %d", *n.TailLines) + + psCmd += `}` + if includeTailLines { + psCmd += fmt.Sprint(` -MaxEvents $Env:kubelet_tailLines`) } - psCmd += " | Sort-Object TimeCreated" - if len(n.Pattern) > 0 { - psCmd += fmt.Sprintf(" | Where-Object -Property Message -Match '%s'", n.Pattern) + psCmd += ` | Sort-Object TimeCreated` + + if includePattern { + psCmd += fmt.Sprintf(` | Where-Object -Property Message -Match "$Env:kubelet_pattern"`) } - psCmd += " | Format-Table -AutoSize -Wrap" + psCmd += ` | Format-Table -AutoSize -Wrap` args = append(args, psCmd) - return powershellExe, args, nil + return args +} + +// getLoggingCmdEnv returns the environment variables that will be present when powershellExe is executed +func getLoggingCmdEnv(n *nodeLogQuery, services []string) (cmdEnv []string) { + if n.SinceTime != nil { + cmdEnv = append(cmdEnv, fmt.Sprintf("kubelet_sinceTime=%s", n.SinceTime.Format(dateLayout))) + } + if n.UntilTime != nil { + cmdEnv = append(cmdEnv, fmt.Sprintf("kubelet_untilTime=%s", n.UntilTime.Format(dateLayout))) + } + + for i, service := range services { + if len(service) > 0 { + cmdEnv = append(cmdEnv, fmt.Sprintf("kubelet_provider%d=%s", i, service)) + } + } + + if n.TailLines != nil { + cmdEnv = append(cmdEnv, fmt.Sprintf("kubelet_tailLines=%d", *n.TailLines)) + } + + if len(n.Pattern) > 0 { + cmdEnv = append(cmdEnv, fmt.Sprintf("kubelet_pattern=%s", n.Pattern)) + } + + return cmdEnv } // checkForNativeLogger always returns true for Windows diff --git a/staging/src/k8s.io/kubelet/config/v1beta1/types.go b/staging/src/k8s.io/kubelet/config/v1beta1/types.go index e5ac2d18f5a..9fe75d41b0c 100644 --- a/staging/src/k8s.io/kubelet/config/v1beta1/types.go +++ b/staging/src/k8s.io/kubelet/config/v1beta1/types.go @@ -726,6 +726,8 @@ type KubeletConfiguration struct { EnableSystemLogHandler *bool `json:"enableSystemLogHandler,omitempty"` // enableSystemLogQuery enables the node log query feature on the /logs endpoint. // EnableSystemLogHandler has to be enabled in addition for this feature to work. + // Enabling this feature has security implications. The recommendation is to enable it on a need basis for debugging + // purposes and disabling otherwise. // Default: false // +featureGate=NodeLogQuery // +optional