From 0655313ad6eda6955272cee239cc527851093250 Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Thu, 16 Jan 2025 21:10:40 +0000 Subject: [PATCH] backend: Prevent flashing terminal on Windows On Windows, even when the exec plugin's command is not interactive, running the command was still making the terminal window flash (default behavior on Windows). This can be annoying if a tool runs the exec plugin frequently. This patch prevents that problem by setting a sys proc attribute to hide windows, only when in the Windows platform and when the exec is non-interactive. Signed-off-by: Joaquim Rocha --- app/electron/main.ts | 1 + backend/pkg/exec/exec.go | 557 ++++++++++++++++++++++++ backend/pkg/exec/syscallattr_other.go | 13 + backend/pkg/exec/syscallattr_windows.go | 15 + backend/pkg/kubeconfig/kubeconfig.go | 46 +- 5 files changed, 630 insertions(+), 2 deletions(-) create mode 100644 backend/pkg/exec/exec.go create mode 100644 backend/pkg/exec/syscallattr_other.go create mode 100644 backend/pkg/exec/syscallattr_windows.go diff --git a/app/electron/main.ts b/app/electron/main.ts index 4b1e0a1ac5..3ec741a71c 100644 --- a/app/electron/main.ts +++ b/app/electron/main.ts @@ -568,6 +568,7 @@ async function startServer(flags: string[] = []): Promise 0 { + // only print extra ": " if the user actually specified a message + suffix = fmt.Sprintf(": %s", config.StdinUnavailableMessage) + } + return false, fmt.Errorf("standard input is unavailable%s", suffix) + } + shouldBeInteractive = true + default: + return false, fmt.Errorf("unknown interactiveMode: %q", config.InteractiveMode) + } + + return shouldBeInteractive, nil +} + +// Authenticator is a client credential provider that rotates credentials by executing a plugin. +// The plugin input and output are defined by the API group client.authentication.k8s.io. +type Authenticator struct { + // Set by the config + cmd string + args []string + group schema.GroupVersion + env []string + cluster *clientauthentication.Cluster + provideClusterInfo bool + + // Used to avoid log spew by rate limiting install hint printing. We didn't do + // this by interval based rate limiting alone since that way may have prevented + // the install hint from showing up for kubectl users. + sometimes *sometimes + installHint string + + // Stubbable for testing + stdin io.Reader + stderr io.Writer + interactiveFunc func() (bool, error) + now func() time.Time + environ func() []string + + // connTracker tracks all connections opened that we need to close when rotating a client certificate + connTracker *connrotation.ConnectionTracker + + // Cached results. + // + // The mutex also guards calling the plugin. Since the plugin could be + // interactive we want to make sure it's only called once. + mu sync.Mutex + cachedCreds *credentials + exp time.Time + + // getCert makes Authenticator.cert comparable to support TLS config caching + getCert *transport.GetCertHolder + // dial is used for clients which do not specify a custom dialer + // it is comparable to support TLS config caching + dial *transport.DialHolder +} + +type credentials struct { + token string `datapolicy:"token"` + cert *tls.Certificate `datapolicy:"secret-key"` +} + +// UpdateTransportConfig updates the transport.Config to use credentials +// returned by the plugin. +func (a *Authenticator) UpdateTransportConfig(c *transport.Config) error { + // If a bearer token is present in the request - avoid the GetCert callback when + // setting up the transport, as that triggers the exec action if the server is + // also configured to allow client certificates for authentication. For requests + // like "kubectl get --token (token) pods" we should assume the intention is to + // use the provided token for authentication. The same can be said for when the + // user specifies basic auth or cert auth. + if c.HasTokenAuth() || c.HasBasicAuth() || c.HasCertAuth() { + return nil + } + + c.Wrap(func(rt http.RoundTripper) http.RoundTripper { + return &roundTripper{a, rt} + }) + if c.HasCertCallback() { + return errors.New("can't add TLS certificate callback: transport.Config.TLS.GetCert already set") + } + c.TLS.GetCertHolder = a.getCert // comparable for TLS config caching + if c.DialHolder != nil { + if c.DialHolder.Dial == nil { + return errors.New("invalid transport.Config.DialHolder: wrapped Dial function is nil") + } + + // if c has a custom dialer, we have to wrap it + // TLS config caching is not supported for this config + d := connrotation.NewDialerWithTracker(c.DialHolder.Dial, a.connTracker) + c.DialHolder = &transport.DialHolder{Dial: d.DialContext} + } else { + c.DialHolder = a.dial // comparable for TLS config caching + } + + return nil +} + +var _ utilnet.RoundTripperWrapper = &roundTripper{} + +type roundTripper struct { + a *Authenticator + base http.RoundTripper +} + +func (r *roundTripper) WrappedRoundTripper() http.RoundTripper { + return r.base +} + +func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + // If a user has already set credentials, use that. This makes commands like + // "kubectl get --token (token) pods" work. + if req.Header.Get("Authorization") != "" { + return r.base.RoundTrip(req) + } + creds, err := r.a.getCreds() + if err != nil { + return nil, fmt.Errorf("getting credentials: %v", err) + } + if creds.token != "" { + req.Header.Set("Authorization", "Bearer "+creds.token) + } + + res, err := r.base.RoundTrip(req) + if err != nil { + return nil, err + } + if res.StatusCode == http.StatusUnauthorized { + if err := r.a.maybeRefreshCreds(creds); err != nil { + klog.Errorf("refreshing credentials: %v", err) + } + } + return res, nil +} + +func (a *Authenticator) credsExpired() bool { + if a.exp.IsZero() { + return false + } + return a.now().After(a.exp) +} + +func (a *Authenticator) cert() (*tls.Certificate, error) { + creds, err := a.getCreds() + if err != nil { + return nil, err + } + return creds.cert, nil +} + +func (a *Authenticator) getCreds() (*credentials, error) { + a.mu.Lock() + defer a.mu.Unlock() + + if a.cachedCreds != nil && !a.credsExpired() { + return a.cachedCreds, nil + } + + if err := a.refreshCredsLocked(); err != nil { + return nil, err + } + + return a.cachedCreds, nil +} + +// maybeRefreshCreds executes the plugin to force a rotation of the +// credentials, unless they were rotated already. +func (a *Authenticator) maybeRefreshCreds(creds *credentials) error { + a.mu.Lock() + defer a.mu.Unlock() + + // Since we're not making a new pointer to a.cachedCreds in getCreds, no + // need to do deep comparison. + if creds != a.cachedCreds { + // Credentials already rotated. + return nil + } + + return a.refreshCredsLocked() +} + +// refreshCredsLocked executes the plugin and reads the credentials from +// stdout. It must be called while holding the Authenticator's mutex. +func (a *Authenticator) refreshCredsLocked() error { + interactive, err := a.interactiveFunc() + if err != nil { + return fmt.Errorf("exec plugin cannot support interactive mode: %w", err) + } + + cred := &clientauthentication.ExecCredential{ + Spec: clientauthentication.ExecCredentialSpec{ + Interactive: interactive, + }, + } + if a.provideClusterInfo { + cred.Spec.Cluster = a.cluster + } + + env := append(a.environ(), a.env...) + data, err := runtime.Encode(codecs.LegacyCodec(a.group), cred) + if err != nil { + return fmt.Errorf("encode ExecCredentials: %v", err) + } + env = append(env, fmt.Sprintf("%s=%s", execInfoEnv, data)) + + stdout := &bytes.Buffer{} + cmd := exec.Command(a.cmd, a.args...) + cmd.Env = env + cmd.Stderr = a.stderr + cmd.Stdout = stdout + if interactive { + cmd.Stdin = a.stdin + } else if goruntime.GOOS == "windows" { + // On Windows, if the process is not interactive, we need to hide the + // terminal window, in order to prevent it from flashing when the exec + // plugin. + cmd.SysProcAttr = GetSysProcAttr() + } + + err = cmd.Run() + // incrementCallsMetric(err) + if err != nil { + return a.wrapCmdRunErrorLocked(err) + } + + _, gvk, err := codecs.UniversalDecoder(a.group).Decode(stdout.Bytes(), nil, cred) + if err != nil { + return fmt.Errorf("decoding stdout: %v", err) + } + if gvk.Group != a.group.Group || gvk.Version != a.group.Version { + return fmt.Errorf("exec plugin is configured to use API version %s, plugin returned version %s", + a.group, schema.GroupVersion{Group: gvk.Group, Version: gvk.Version}) + } + + if cred.Status == nil { + return fmt.Errorf("exec plugin didn't return a status field") + } + if cred.Status.Token == "" && cred.Status.ClientCertificateData == "" && cred.Status.ClientKeyData == "" { + return fmt.Errorf("exec plugin didn't return a token or cert/key pair") + } + if (cred.Status.ClientCertificateData == "") != (cred.Status.ClientKeyData == "") { + return fmt.Errorf("exec plugin returned only certificate or key, not both") + } + + if cred.Status.ExpirationTimestamp != nil { + a.exp = cred.Status.ExpirationTimestamp.Time + } else { + a.exp = time.Time{} + } + + newCreds := &credentials{ + token: cred.Status.Token, + } + if cred.Status.ClientKeyData != "" && cred.Status.ClientCertificateData != "" { + cert, err := tls.X509KeyPair([]byte(cred.Status.ClientCertificateData), []byte(cred.Status.ClientKeyData)) + if err != nil { + return fmt.Errorf("failed parsing client key/certificate: %v", err) + } + + // Leaf is initialized to be nil: + // https://golang.org/pkg/crypto/tls/#X509KeyPair + // Leaf certificate is the first certificate: + // https://golang.org/pkg/crypto/tls/#Certificate + // Populating leaf is useful for quickly accessing the underlying x509 + // certificate values. + cert.Leaf, err = x509.ParseCertificate(cert.Certificate[0]) + if err != nil { + return fmt.Errorf("failed parsing client leaf certificate: %v", err) + } + newCreds.cert = &cert + } + + oldCreds := a.cachedCreds + a.cachedCreds = newCreds + // Only close all connections when TLS cert rotates. Token rotation doesn't + // need the extra noise. + if oldCreds != nil && !reflect.DeepEqual(oldCreds.cert, a.cachedCreds.cert) { + // Can be nil if the exec auth plugin only returned token auth. + if oldCreds.cert != nil && oldCreds.cert.Leaf != nil { + metrics.ClientCertRotationAge.Observe(time.Since(oldCreds.cert.Leaf.NotBefore)) + } + a.connTracker.CloseAll() + } + + // if a.cachedCreds.cert != nil && a.cachedCreds.cert.Leaf != nil { + // expiry = a.cachedCreds.cert.Leaf.NotAfter + // } + // expirationMetrics.set(a, expiry) + return nil +} + +// wrapCmdRunErrorLocked pulls out the code to construct a helpful error message +// for when the exec plugin's binary fails to Run(). +// +// It must be called while holding the Authenticator's mutex. +func (a *Authenticator) wrapCmdRunErrorLocked(err error) error { + switch err.(type) { + case *exec.Error: // Binary does not exist (see exec.Error). + builder := strings.Builder{} + fmt.Fprintf(&builder, "exec: executable %s not found", a.cmd) + + a.sometimes.Do(func() { + fmt.Fprint(&builder, installHintVerboseHelp) + if a.installHint != "" { + fmt.Fprintf(&builder, "\n\n%s", a.installHint) + } + }) + + return errors.New(builder.String()) + + case *exec.ExitError: // Binary execution failed (see exec.Cmd.Run()). + e := err.(*exec.ExitError) + return fmt.Errorf( + "exec: executable %s failed with exit code %d", + a.cmd, + e.ProcessState.ExitCode(), + ) + + default: + return fmt.Errorf("exec: %v", err) + } +} diff --git a/backend/pkg/exec/syscallattr_other.go b/backend/pkg/exec/syscallattr_other.go new file mode 100644 index 0000000000..4cc260dd12 --- /dev/null +++ b/backend/pkg/exec/syscallattr_other.go @@ -0,0 +1,13 @@ +//go:build !windows + +package exec + +import ( + "syscall" +) + +func GetSysProcAttr() *syscall.SysProcAttr { + // Nothing here since it's not needed. It's just for keeping the + // multi-platform code working. + return nil +} diff --git a/backend/pkg/exec/syscallattr_windows.go b/backend/pkg/exec/syscallattr_windows.go new file mode 100644 index 0000000000..95e7e1895d --- /dev/null +++ b/backend/pkg/exec/syscallattr_windows.go @@ -0,0 +1,15 @@ +//go:build windows + +package exec + +import ( + "syscall" +) + +func GetSysProcAttr() *syscall.SysProcAttr { + // This prevents the console window from appearing when running the command, + // since we do not want it flashing when the exec plugin is running. + return &syscall.SysProcAttr{ + HideWindow: true, + } +} diff --git a/backend/pkg/kubeconfig/kubeconfig.go b/backend/pkg/kubeconfig/kubeconfig.go index 6419119bfe..5dcbfbf045 100644 --- a/backend/pkg/kubeconfig/kubeconfig.go +++ b/backend/pkg/kubeconfig/kubeconfig.go @@ -17,9 +17,12 @@ import ( "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd/api" + "github.com/headlamp-k8s/headlamp/backend/pkg/exec" "github.com/headlamp-k8s/headlamp/backend/pkg/logger" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" + "k8s.io/client-go/pkg/apis/clientauthentication" + rest "k8s.io/client-go/rest" + "k8s.io/client-go/transport" ) // TODO: Use a different way to avoid name clashes with other clusters. @@ -185,6 +188,45 @@ func (c *Context) RESTConfig() (*rest.Config, error) { return clientConfig.ClientConfig() } +func makeTransportFor(conf *rest.Config) (http.RoundTripper, error) { + // We want to override the Exec related transport config when on Windows to + // make it run without flashing the terminal window. + if conf.ExecProvider == nil || runtime.GOOS != "windows" { + return rest.TransportFor(conf) + } + + confNoExec := *conf + confNoExec.ExecProvider = nil + // Get the Transport Config but without the ExecProvider because we will set + // it up with our version. + cfg, err := confNoExec.TransportConfig() + if err != nil { + return nil, err + } + + var cluster *clientauthentication.Cluster + + if conf.ExecProvider.ProvideClusterInfo { + var err error + + cluster, err = rest.ConfigToExecCluster(conf) + if err != nil { + return nil, err + } + } + + provider, err := exec.GetAuthenticator(conf.ExecProvider, cluster) + if err != nil { + return nil, err + } + + if err := provider.UpdateTransportConfig(cfg); err != nil { + return nil, err + } + + return transport.New(cfg) +} + // OidcConfig returns the oidc config for the context. func (c *Context) OidcConfig() (*OidcConfig, error) { if c.OidcConf != nil { @@ -256,7 +298,7 @@ func (c *Context) SetupProxy() error { restConf, err := c.RESTConfig() if err == nil { - roundTripper, err := rest.TransportFor(restConf) + roundTripper, err := makeTransportFor(restConf) if err == nil { proxy.Transport = roundTripper }