diff --git a/cmd/containerboot/kube.go b/cmd/containerboot/kube.go index 5a0d6d6a4..03eff50c4 100644 --- a/cmd/containerboot/kube.go +++ b/cmd/containerboot/kube.go @@ -21,54 +21,82 @@ import ( "path/filepath" "strings" "time" + + "tailscale.com/util/multierr" ) -// checkSecretPermissions checks that the current pod has permission to read, -// write and patch secretName in the cluster. -func checkSecretPermissions(ctx context.Context, secretName string) error { - for _, verb := range []string{"get", "update", "patch"} { - sar := map[string]any{ - "apiVersion": "authorization.k8s.io/v1", - "kind": "SelfSubjectAccessReview", - "spec": map[string]any{ - "resourceAttributes": map[string]any{ - "namespace": kubeNamespace, - "verb": verb, - "resource": "secrets", - "name": secretName, - }, - }, - } - bs, err := json.Marshal(sar) +// checkSecretPermissions checks the secret access permissions of the current +// pod. It returns an error if the basic permissions tailscale needs are +// missing, and reports whether the patch permission is additionally present. +// +// Errors encountered during the access checking process are logged, but ignored +// so that the pod tries to fail alive if the permissions exist and there's just +// something wrong with SelfSubjectAccessReviews. There shouldn't be, pods +// should always be able to use SSARs to assess their own permissions, but since +// we didn't use to check permissions this way we'll be cautious in case some +// old version of k8s deviates from the current behavior. +func checkSecretPermissions(ctx context.Context, secretName string) (canPatch bool, err error) { + var errs []error + for _, verb := range []string{"get", "update"} { + ok, err := checkPermission(ctx, verb, secretName) if err != nil { - return err - } - req, err := http.NewRequest("POST", "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", bytes.NewReader(bs)) - if err != nil { - return err - } - resp, err := doKubeRequest(ctx, req) - if err != nil { - return err - } - defer resp.Body.Close() - bs, err = io.ReadAll(resp.Body) - if err != nil { - return err - } - var res struct { - Status struct { - Allowed bool `json:"allowed"` - } `json:"status"` - } - if err := json.Unmarshal(bs, &res); err != nil { - return err - } - if !res.Status.Allowed { - return fmt.Errorf("missing permission: cannot %s secret %q", verb, secretName) + log.Printf("error checking %s permission on secret %s: %v", verb, secretName, err) + } else if !ok { + errs = append(errs, fmt.Errorf("missing %s permission on secret %q", verb, secretName)) } } - return nil + if len(errs) > 0 { + return false, multierr.New(errs...) + } + ok, err := checkPermission(ctx, "patch", secretName) + if err != nil { + log.Printf("error checking patch permission on secret %s: %v", secretName, err) + return false, nil + } + return ok, nil +} + +// checkPermission reports whether the current pod has permission to use the +// given verb (e.g. get, update, patch) on secretName. +func checkPermission(ctx context.Context, verb, secretName string) (bool, error) { + sar := map[string]any{ + "apiVersion": "authorization.k8s.io/v1", + "kind": "SelfSubjectAccessReview", + "spec": map[string]any{ + "resourceAttributes": map[string]any{ + "namespace": kubeNamespace, + "verb": verb, + "resource": "secrets", + "name": secretName, + }, + }, + } + bs, err := json.Marshal(sar) + if err != nil { + return false, err + } + req, err := http.NewRequest("POST", "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", bytes.NewReader(bs)) + if err != nil { + return false, err + } + resp, err := doKubeRequest(ctx, req) + if err != nil { + return false, err + } + defer resp.Body.Close() + bs, err = io.ReadAll(resp.Body) + if err != nil { + return false, err + } + var res struct { + Status struct { + Allowed bool `json:"allowed"` + } `json:"status"` + } + if err := json.Unmarshal(bs, &res); err != nil { + return false, err + } + return res.Status.Allowed, nil } // findKeyInKubeSecret inspects the kube secret secretName for a data diff --git a/cmd/containerboot/main.go b/cmd/containerboot/main.go index 583de3e3a..1e92f5237 100644 --- a/cmd/containerboot/main.go +++ b/cmd/containerboot/main.go @@ -117,15 +117,28 @@ func main() { defer cancel() if cfg.InKubernetes && cfg.KubeSecret != "" { - if err := checkSecretPermissions(ctx, cfg.KubeSecret); err != nil { + canPatch, err := checkSecretPermissions(ctx, cfg.KubeSecret) + if err != nil { log.Fatalf("Some Kubernetes permissions are missing, please check your RBAC configuration: %v", err) } + cfg.KubernetesCanPatch = canPatch + if cfg.AuthKey == "" { key, err := findKeyInKubeSecret(ctx, cfg.KubeSecret) if err != nil { log.Fatalf("Getting authkey from kube secret: %v", err) } if key != "" { + // This behavior of pulling authkeys from kube secrets was added + // at the same time as the patch permission, so we can enforce + // that we must be able to patch out the authkey after + // authenticating if you want to use this feature. This avoids + // us having to deal with the case where we might leave behind + // an unnecessary reusable authkey in a secret, like a rake in + // the grass. + if !cfg.KubernetesCanPatch { + log.Fatalf("authkey found in TS_KUBE_SECRET, but the pod doesn't have patch permissions on the secret to manage the authkey.") + } log.Print("Using authkey found in kube secret") cfg.AuthKey = key } else { @@ -149,7 +162,7 @@ func main() { log.Fatalf("installing proxy rules: %v", err) } } - if cfg.InKubernetes && cfg.KubeSecret != "" { + if cfg.InKubernetes && cfg.KubernetesCanPatch && cfg.KubeSecret != "" { if err := storeDeviceID(ctx, cfg.KubeSecret, string(st.Self.ID)); err != nil { log.Fatalf("storing device ID in kube secret: %v", err) } @@ -446,21 +459,22 @@ func installIPTablesRule(ctx context.Context, dstStr string, tsIPs []netip.Addr) // settings is all the configuration for containerboot. type settings struct { - AuthKey string - Routes string - ProxyTo string - DaemonExtraArgs string - ExtraArgs string - InKubernetes bool - UserspaceMode bool - StateDir string - AcceptDNS bool - KubeSecret string - SOCKSProxyAddr string - HTTPProxyAddr string - Socket string - AuthOnce bool - Root string + AuthKey string + Routes string + ProxyTo string + DaemonExtraArgs string + ExtraArgs string + InKubernetes bool + UserspaceMode bool + StateDir string + AcceptDNS bool + KubeSecret string + SOCKSProxyAddr string + HTTPProxyAddr string + Socket string + AuthOnce bool + Root string + KubernetesCanPatch bool } // defaultEnv returns the value of the given envvar name, or defVal if diff --git a/cmd/containerboot/main_test.go b/cmd/containerboot/main_test.go index 95a616e32..8fb7c6c9f 100644 --- a/cmd/containerboot/main_test.go +++ b/cmd/containerboot/main_test.go @@ -114,10 +114,11 @@ func TestContainerBoot(t *testing.T) { WantFiles map[string]string } tests := []struct { - Name string - Env map[string]string - KubeSecret map[string]string - Phases []phase + Name string + Env map[string]string + KubeSecret map[string]string + KubeDenyPatch bool + Phases []phase }{ { // Out of the box default: runs in userspace mode, ephemeral storage, interactive login. @@ -370,6 +371,35 @@ func TestContainerBoot(t *testing.T) { }, }, }, + { + Name: "kube_storage_no_patch", + Env: map[string]string{ + "KUBERNETES_SERVICE_HOST": kube.Host, + "KUBERNETES_SERVICE_PORT_HTTPS": kube.Port, + "TS_AUTH_KEY": "tskey-key", + }, + KubeSecret: map[string]string{}, + KubeDenyPatch: true, + Phases: []phase{ + { + WantCmds: []string{ + "/usr/bin/tailscaled --socket=/tmp/tailscaled.sock --state=kube:tailscale --statedir=/tmp --tun=userspace-networking", + "/usr/bin/tailscale --socket=/tmp/tailscaled.sock up --accept-dns=false --authkey=tskey-key", + }, + WantKubeSecret: map[string]string{}, + }, + { + Status: ipnstate.Status{ + BackendState: "Running", + TailscaleIPs: tsIPs, + Self: &ipnstate.PeerStatus{ + ID: tailcfg.StableNodeID("myID"), + }, + }, + WantKubeSecret: map[string]string{}, + }, + }, + }, { // Same as previous, but deletes the authkey from the kube secret. Name: "kube_storage_auth_once", @@ -492,6 +522,7 @@ func TestContainerBoot(t *testing.T) { for k, v := range test.KubeSecret { kube.SetSecret(k, v) } + kube.SetPatching(!test.KubeDenyPatch) cmd := exec.Command(boot) cmd.Env = []string{ @@ -722,7 +753,8 @@ type kubeServer struct { srv *httptest.Server sync.Mutex - secret map[string]string + secret map[string]string + canPatch bool } func (k *kubeServer) Secret() map[string]string { @@ -741,6 +773,12 @@ func (k *kubeServer) SetSecret(key, val string) { k.secret[key] = val } +func (k *kubeServer) SetPatching(canPatch bool) { + k.Lock() + defer k.Unlock() + k.canPatch = canPatch +} + func (k *kubeServer) Reset() { k.Lock() defer k.Unlock() @@ -784,16 +822,39 @@ func (k *kubeServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Header.Get("Authorization") != "Bearer bearer_token" { panic("client didn't provide bearer token in request") } - if r.URL.Path == "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews" { - // Just say yes to all SARs, we don't enforce RBAC. - w.Header().Set("Content-Type", "application/json") - fmt.Fprintln(w, `{"status":{"allowed":true}}`) - return - } - if r.URL.Path != "/api/v1/namespaces/default/secrets/tailscale" { + switch r.URL.Path { + case "/api/v1/namespaces/default/secrets/tailscale": + k.serveSecret(w, r) + case "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews": + k.serveSSAR(w, r) + default: panic(fmt.Sprintf("unhandled fake kube api path %q", r.URL.Path)) } +} +func (k *kubeServer) serveSSAR(w http.ResponseWriter, r *http.Request) { + var req struct { + Spec struct { + ResourceAttributes struct { + Verb string `json:"verb"` + } `json:"resourceAttributes"` + } `json:"spec"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + panic(fmt.Sprintf("decoding SSAR request: %v", err)) + } + ok := true + if req.Spec.ResourceAttributes.Verb == "patch" { + k.Lock() + defer k.Unlock() + ok = k.canPatch + } + // Just say yes to all SARs, we don't enforce RBAC. + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"status":{"allowed":%v}}`, ok) +} + +func (k *kubeServer) serveSecret(w http.ResponseWriter, r *http.Request) { bs, err := io.ReadAll(r.Body) if err != nil { http.Error(w, fmt.Sprintf("reading request body: %v", err), http.StatusInternalServerError) @@ -819,6 +880,11 @@ func (k *kubeServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { panic("encode failed") } case "PATCH": + k.Lock() + defer k.Unlock() + if !k.canPatch { + panic("containerboot tried to patch despite not being allowed") + } switch r.Header.Get("Content-Type") { case "application/json-patch+json": req := []struct { @@ -828,8 +894,6 @@ func (k *kubeServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err := json.Unmarshal(bs, &req); err != nil { panic(fmt.Sprintf("json decode failed: %v. Body:\n\n%s", err, string(bs))) } - k.Lock() - defer k.Unlock() for _, op := range req { if op.Op != "remove" { panic(fmt.Sprintf("unsupported json-patch op %q", op.Op)) @@ -846,8 +910,6 @@ func (k *kubeServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err := json.Unmarshal(bs, &req); err != nil { panic(fmt.Sprintf("json decode failed: %v. Body:\n\n%s", err, string(bs))) } - k.Lock() - defer k.Unlock() for key, val := range req.Data { k.secret[key] = val }