diff --git a/sdk/clusterplugin/plugin.go b/sdk/clusterplugin/plugin.go index 9b9cab4921..17861a0ddc 100644 --- a/sdk/clusterplugin/plugin.go +++ b/sdk/clusterplugin/plugin.go @@ -2,12 +2,16 @@ package clusterplugin import ( "encoding/json" + "errors" "fmt" "os" + "path/filepath" + "syscall" "github.com/kairos-io/kairos/v4/sdk/bus" "github.com/mudler/go-pluggable" yip "github.com/mudler/yip/pkg/schema" + "github.com/twpayne/go-vfs/v4" "gopkg.in/yaml.v3" ) @@ -55,30 +59,108 @@ func (p ClusterPlugin) onBoot(event *pluggable.Event) pluggable.EventResponse { configFilePath = config.Cluster.ClusterConfigPath } - // open our cloud configuration file for writing - f, err := filesystem.OpenFile(configFilePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) - if err != nil { - response.Error = fmt.Sprintf("failed to parse boot event: %s", err.Error()) + if err := writeClusterConfig(configFilePath, cc); err != nil { + response.Error = err.Error() return response } + return response +} + +// writeClusterConfig writes the provider's cloud-config to configFilePath. +// +// The file holds the cluster token in cleartext and is written as root on +// every boot, at a path on the persistent partition, so the open has to +// survive whatever is sitting there already (kairos-io/kairos#4867): +// +// - the parent directory is created first. A provider that points +// cluster_config_path at a directory the image does not ship, or a node +// whose /usr/local/cloud-config has not been created yet, used to fail the +// whole boot event on ENOENT. +// - O_NOFOLLOW refuses a symlink at the last path element. Without it a +// symlink planted there redirects the write, and O_TRUNC empties whatever +// it points at. Only the last element is protected, so a symlinked +// cloud-config directory keeps working. +// - O_NONBLOCK returns instead of parking in open(2) when the path is a FIFO +// with no reader, and the fstat that follows rejects every file type that +// is not a regular file. The flag does not change writes to a regular file. +// - the mode is set after the open. O_CREATE applies perm only when it +// creates the file, so a file pre-created by another user kept its own mode +// and then received the cluster token. +// +// Planting any of these needs write access to a persistent path, so this is +// robustness rather than a privilege boundary. What it buys is that the failure +// is reported, instead of being a truncated stranger's file, a boot that hangs +// forever, or a token anyone can read. +func writeClusterConfig(configFilePath string, cc yip.YipConfig) error { + dir := filepath.Dir(configFilePath) + if err := vfs.MkdirAll(filesystem, dir, 0700); err != nil { + return fmt.Errorf("failed to create the cluster config directory %s: %w", dir, err) + } + + f, err := filesystem.OpenFile(configFilePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0600) + if err != nil { + // Name what is in the way. Both of these arrive as a bare errno that + // says nothing about the path holding something other than a file. + switch { + case errors.Is(err, syscall.ELOOP): + return fmt.Errorf("refusing to write the cluster config to %s: it is a symlink, not a regular file", configFilePath) + case errors.Is(err, syscall.ENXIO): + return fmt.Errorf("refusing to write the cluster config to %s: it is a named pipe, not a regular file", configFilePath) + } + + return fmt.Errorf("failed to open the cluster config file %s: %w", configFilePath, err) + } + defer f.Close() - // write the cloud configuration header - _, err = f.WriteString("#cloud-config\n") + stat, err := f.Stat() if err != nil { - response.Error = fmt.Sprintf("failed to parse boot event: %s", err.Error()) - return response + return fmt.Errorf("failed to stat the cluster config file %s: %w", configFilePath, err) + } + + if !stat.Mode().IsRegular() { + return fmt.Errorf("refusing to write the cluster config to %s: it is %s, not a regular file", configFilePath, fileTypeName(stat.Mode())) + } + + if stat.Mode().Perm() != 0600 { + if err := f.Chmod(0600); err != nil { + return fmt.Errorf("failed to restrict the cluster config file %s to 0600: %w", configFilePath, err) + } + } + + // write the cloud configuration header + if _, err := f.WriteString("#cloud-config\n"); err != nil { + return fmt.Errorf("failed to write the cluster config file %s: %w", configFilePath, err) } // encode the provider's configuration - err = yaml.NewEncoder(f).Encode(cc) - if err != nil { - response.Error = fmt.Sprintf("failed to parse boot event: %s", err.Error()) - return response + if err := yaml.NewEncoder(f).Encode(cc); err != nil { + return fmt.Errorf("failed to encode the cluster config into %s: %w", configFilePath, err) } - return response + return nil +} + +// fileTypeName names the file type in an error somebody has to act on, since +// "not a regular file" on its own does not say what to go and look for. +func fileTypeName(mode os.FileMode) string { + switch { + case mode&os.ModeNamedPipe != 0: + return "a named pipe" + case mode&os.ModeSocket != 0: + return "a socket" + case mode&os.ModeCharDevice != 0: + return "a character device" + case mode&os.ModeDevice != 0: + return "a block device" + case mode&os.ModeSymlink != 0: + return "a symlink" + case mode.IsDir(): + return "a directory" + default: + return "an irregular file" + } } func (p ClusterPlugin) Run(extraPlugins ...pluggable.FactoryPlugin) error { diff --git a/sdk/clusterplugin/plugin_test.go b/sdk/clusterplugin/plugin_test.go new file mode 100644 index 0000000000..776f6a8a60 --- /dev/null +++ b/sdk/clusterplugin/plugin_test.go @@ -0,0 +1,212 @@ +package clusterplugin + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "syscall" + "time" + + "github.com/kairos-io/kairos/v4/sdk/bus" + "github.com/mudler/go-pluggable" + yip "github.com/mudler/yip/pkg/schema" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// The cluster config the provider hands back. Its content only has to be +// recognisable in the file afterwards. +func testProviderConfig() yip.YipConfig { + return yip.YipConfig{Name: "cluster-token-in-here"} +} + +// bootEvent builds the payload agent.boot sends, with cluster_config_path +// pointing at path. +func bootEvent(path string) *pluggable.Event { + cfg := fmt.Sprintf("#cloud-config\ncluster:\n cluster_token: sekret\n role: worker\n cluster_config_path: %s\n", path) + data, err := json.Marshal(bus.EventPayload{Config: cfg}) + Expect(err).ToNot(HaveOccurred()) + + return &pluggable.Event{Name: bus.EventBoot, Data: string(data)} +} + +// callWithin runs f and fails the spec if it has not returned by bound, so +// that a blocking open fails the run instead of hanging it forever. +func callWithin(bound time.Duration, f func() error) error { + done := make(chan error, 1) + go func() { + defer GinkgoRecover() + done <- f() + }() + + select { + case err := <-done: + return err + case <-time.After(bound): + Fail(fmt.Sprintf("writeClusterConfig did not return within %s", bound)) + return nil + } +} + +var _ = Describe("writeClusterConfig", func() { + var root string + + BeforeEach(func() { + root = GinkgoT().TempDir() + }) + + It("creates the directory the cluster config goes in", func() { + // A node whose /usr/local/cloud-config does not exist yet used to + // fail the whole boot event here, on ENOENT. + path := filepath.Join(root, "cloud-config", "cluster.kairos.yaml") + + Expect(writeClusterConfig(path, testProviderConfig())).To(Succeed()) + + content, err := os.ReadFile(path) + Expect(err).ToNot(HaveOccurred()) + Expect(string(content)).To(HavePrefix("#cloud-config\n")) + Expect(string(content)).To(ContainSubstring("cluster-token-in-here")) + + info, err := os.Stat(path) + Expect(err).ToNot(HaveOccurred()) + Expect(info.Mode().Perm()).To(Equal(os.FileMode(0600))) + }) + + It("writes the config once the directory is already there", func() { + path := filepath.Join(root, "cluster.kairos.yaml") + + Expect(writeClusterConfig(path, testProviderConfig())).To(Succeed()) + // A second boot rewrites it in place. + Expect(writeClusterConfig(path, testProviderConfig())).To(Succeed()) + + content, err := os.ReadFile(path) + Expect(err).ToNot(HaveOccurred()) + Expect(string(content)).To(HavePrefix("#cloud-config\n")) + }) + + It("refuses a symlink at the config path, and leaves its target alone", func() { + victim := filepath.Join(root, "victim") + Expect(os.WriteFile(victim, []byte("do not truncate me"), 0644)).To(Succeed()) + + path := filepath.Join(root, "cluster.kairos.yaml") + Expect(os.Symlink(victim, path)).To(Succeed()) + + err := writeClusterConfig(path, testProviderConfig()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(path)) + Expect(err.Error()).To(ContainSubstring("it is a symlink")) + + // O_TRUNC never reached the target. + content, err := os.ReadFile(victim) + Expect(err).ToNot(HaveOccurred()) + Expect(string(content)).To(Equal("do not truncate me")) + }) + + It("does not block on a named pipe nobody is reading", func() { + path := filepath.Join(root, "cluster.kairos.yaml") + Expect(syscall.Mkfifo(path, 0600)).To(Succeed()) + + err := callWithin(10*time.Second, func() error { + return writeClusterConfig(path, testProviderConfig()) + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("it is a named pipe")) + }) + + It("refuses a named pipe somebody is reading, instead of writing the token into it", func() { + path := filepath.Join(root, "cluster.kairos.yaml") + Expect(syscall.Mkfifo(path, 0600)).To(Succeed()) + + // A reader makes the write-side open succeed, so only the fstat + // stands between the cluster token and the pipe. + r, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) + Expect(err).ToNot(HaveOccurred()) + defer r.Close() + + err = callWithin(10*time.Second, func() error { + return writeClusterConfig(path, testProviderConfig()) + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("a named pipe")) + Expect(err.Error()).To(ContainSubstring("not a regular file")) + + // Nothing was handed to the pipe. The write side is closed again by + // now, so the read reports EOF rather than EAGAIN; either way it + // carries no bytes. + buf := make([]byte, 4096) + n, _ := r.Read(buf) + Expect(n).To(Equal(0)) + }) + + It("refuses a directory at the config path", func() { + path := filepath.Join(root, "cluster.kairos.yaml") + Expect(os.Mkdir(path, 0700)).To(Succeed()) + + err := writeClusterConfig(path, testProviderConfig()) + Expect(err).To(HaveOccurred()) + }) + + It("takes the mode down to 0600 on a file somebody else created", func() { + // O_CREATE applies its perm argument only when it creates the file, + // so a pre-created 0666 file used to receive the cluster token and + // stay world readable. + path := filepath.Join(root, "cluster.kairos.yaml") + Expect(os.WriteFile(path, []byte("planted"), 0666)).To(Succeed()) + Expect(os.Chmod(path, 0666)).To(Succeed()) + + Expect(writeClusterConfig(path, testProviderConfig())).To(Succeed()) + + info, err := os.Stat(path) + Expect(err).ToNot(HaveOccurred()) + Expect(info.Mode().Perm()).To(Equal(os.FileMode(0600))) + + content, err := os.ReadFile(path) + Expect(err).ToNot(HaveOccurred()) + Expect(string(content)).ToNot(ContainSubstring("planted")) + }) +}) + +var _ = Describe("onBoot", func() { + var plugin ClusterPlugin + + BeforeEach(func() { + plugin = ClusterPlugin{Provider: func(cluster Cluster) yip.YipConfig { + return testProviderConfig() + }} + }) + + It("says what went wrong with the file, not that the event failed to parse", func() { + // Every error past the JSON unmarshal used to read + // "failed to parse boot event", which sends the reader to the + // payload rather than to the filesystem. + root := GinkgoT().TempDir() + path := filepath.Join(root, "cluster.kairos.yaml") + Expect(os.Mkdir(path, 0700)).To(Succeed()) + + response := plugin.onBoot(bootEvent(path)) + + Expect(response.Error).ToNot(BeEmpty()) + Expect(response.Error).ToNot(ContainSubstring("failed to parse boot event")) + Expect(response.Error).To(ContainSubstring(path)) + }) + + It("still reports a payload that is not JSON as a parse failure", func() { + response := plugin.onBoot(&pluggable.Event{Name: bus.EventBoot, Data: "not json"}) + + Expect(response.Error).To(ContainSubstring("failed to parse boot event")) + }) + + It("writes the provider config through to the path the cluster asked for", func() { + root := GinkgoT().TempDir() + path := filepath.Join(root, "cloud-config", "cluster.kairos.yaml") + + response := plugin.onBoot(bootEvent(path)) + Expect(response.Error).To(BeEmpty()) + + content, err := os.ReadFile(path) + Expect(err).ToNot(HaveOccurred()) + Expect(string(content)).To(ContainSubstring("cluster-token-in-here")) + }) +}) diff --git a/sdk/clusterplugin/suite_test.go b/sdk/clusterplugin/suite_test.go new file mode 100644 index 0000000000..68abb30c59 --- /dev/null +++ b/sdk/clusterplugin/suite_test.go @@ -0,0 +1,13 @@ +package clusterplugin + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestClusterPlugin(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Cluster Plugin Suite") +}