diff --git a/internal/helm/chart/secureloader/directory.go b/internal/helm/chart/secureloader/directory.go new file mode 100644 index 00000000..6b342a68 --- /dev/null +++ b/internal/helm/chart/secureloader/directory.go @@ -0,0 +1,208 @@ +/* +Copyright The Helm Authors. +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +This file has been derived from +https://github.com/helm/helm/blob/v3.8.1/pkg/chart/loader/directory.go. + +It has been modified to not blindly accept any resolved symlink path, but +instead check it against the configured root before allowing it to be included. +It also allows for capping the size of any file loaded into the chart. +*/ + +package secureloader + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "strings" + + securejoin "github.com/cyphar/filepath-securejoin" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" + + "github.com/fluxcd/source-controller/internal/helm/chart/secureloader/ignore" + "github.com/fluxcd/source-controller/internal/helm/chart/secureloader/sympath" +) + +var ( + // DefaultMaxFileSize is the default maximum file size of any chart file + // loaded. + DefaultMaxFileSize = 16 << 20 // 16MiB + + utf8bom = []byte{0xEF, 0xBB, 0xBF} +) + +// SecureDirLoader securely loads a chart from a directory while resolving +// symlinks without including files outside root. +type SecureDirLoader struct { + root string + dir string + maxSize int +} + +// NewSecureDirLoader returns a new SecureDirLoader, configured to the scope of the +// root and provided dir. Max size configures the maximum size a file must not +// exceed to be loaded. If 0 it defaults to defaultMaxFileSize, it can be +// disabled using a negative integer. +func NewSecureDirLoader(root string, dir string, maxSize int) SecureDirLoader { + if maxSize == 0 { + maxSize = DefaultMaxFileSize + } + return SecureDirLoader{ + root: root, + dir: dir, + maxSize: maxSize, + } +} + +// Load loads and returns the chart.Chart, or an error. +func (l SecureDirLoader) Load() (*chart.Chart, error) { + return SecureLoadDir(l.root, l.dir, l.maxSize) +} + +// SecureLoadDir securely loads from a directory, without going outside root. +func SecureLoadDir(root, dir string, maxSize int) (*chart.Chart, error) { + root, err := filepath.Abs(root) + if err != nil { + return nil, err + } + + topDir, err := filepath.Abs(dir) + if err != nil { + return nil, err + } + + // Confirm topDir is actually relative to root + if _, err = isSecureSymlinkPath(root, topDir); err != nil { + return nil, fmt.Errorf("cannot load chart from dir: %w", err) + } + + // Just used for errors + c := &chart.Chart{} + + // Get the absolute location of the .helmignore file + relDirPath, err := filepath.Rel(root, topDir) + if err != nil { + // We are not expected to be returning this error, as the above call to + // isSecureSymlinkPath already does the same. However, especially + // because we are dealing with security aspects here, we check it + // anyway in case this assumption changes. + return nil, err + } + iFile, err := securejoin.SecureJoin(root, filepath.Join(relDirPath, ignore.HelmIgnore)) + + // Load the .helmignore rules + rules := ignore.Empty() + if _, err = os.Stat(iFile); err == nil { + r, err := ignore.ParseFile(iFile) + if err != nil { + return c, err + } + rules = r + } + rules.AddDefaults() + + var files []*loader.BufferedFile + topDir += string(filepath.Separator) + + walk := func(name, absoluteName string, fi os.FileInfo, err error) error { + n := strings.TrimPrefix(name, topDir) + if n == "" { + // No need to process top level. Avoid bug with helmignore .* matching + // empty names. See issue 1779. + return nil + } + + // Normalize to / since it will also work on Windows + n = filepath.ToSlash(n) + + if err != nil { + return err + } + if fi.IsDir() { + // Directory-based ignore rules should involve skipping the entire + // contents of that directory. + if rules.Ignore(n, fi) { + return filepath.SkipDir + } + // Check after excluding ignores to provide the user with an option + // to opt-out from including certain paths. + if _, err := isSecureSymlinkPath(root, absoluteName); err != nil { + return fmt.Errorf("cannot load '%s' directory: %w", n, err) + } + return nil + } + + // If a .helmignore file matches, skip this file. + if rules.Ignore(n, fi) { + return nil + } + + // Check after excluding ignores to provide the user with an option + // to opt-out from including certain paths. + if _, err := isSecureSymlinkPath(root, absoluteName); err != nil { + return fmt.Errorf("cannot load '%s' file: %w", n, err) + } + + // Irregular files include devices, sockets, and other uses of files that + // are not regular files. In Go they have a file mode type bit set. + // See https://golang.org/pkg/os/#FileMode for examples. + if !fi.Mode().IsRegular() { + return fmt.Errorf("cannot load irregular file %s as it has file mode type bits set", n) + } + + if fileSize := fi.Size(); maxSize > 0 && fileSize > int64(maxSize) { + return fmt.Errorf("cannot load file %s as file size (%d) exceeds limit (%d)", n, fileSize, maxSize) + } + + data, err := os.ReadFile(name) + if err != nil { + return fmt.Errorf("error reading %s: %w", n, err) + } + data = bytes.TrimPrefix(data, utf8bom) + + files = append(files, &loader.BufferedFile{Name: n, Data: data}) + return nil + } + if err = sympath.Walk(topDir, walk); err != nil { + return c, err + } + return loader.LoadFiles(files) +} + +// isSecureSymlinkPath attempts to make the given absolute path relative to +// root and securely joins this with root. If the result equals absolute path, +// it is safe to use. +func isSecureSymlinkPath(root, absPath string) (bool, error) { + root, absPath = filepath.Clean(root), filepath.Clean(absPath) + if root == "/" { + return true, nil + } + unsafePath, err := filepath.Rel(root, absPath) + if err != nil { + return false, fmt.Errorf("cannot calculate path relative to root for resolved symlink") + } + safePath, err := securejoin.SecureJoin(root, unsafePath) + if err != nil { + return false, fmt.Errorf("cannot securely join root with resolved relative symlink path") + } + if safePath != absPath { + return false, fmt.Errorf("symlink traverses outside root boundary: relative path to root %s", unsafePath) + } + return true, nil +} diff --git a/internal/helm/chart/secureloader/directory_test.go b/internal/helm/chart/secureloader/directory_test.go new file mode 100644 index 00000000..e2031062 --- /dev/null +++ b/internal/helm/chart/secureloader/directory_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package secureloader + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func Test_isSecureSymlinkPath(t *testing.T) { + tests := []struct { + name string + root string + absPath string + safe bool + wantErr string + }{ + { + name: "absolute path in root", + root: "/", + absPath: "/bar/", + safe: true, + }, + + { + name: "abs path not relative to root", + root: "/working/dir", + absPath: "/working/in/another/dir", + safe: false, + wantErr: "symlink traverses outside root boundary", + }, + { + name: "abs path relative to root", + root: "/working/dir/", + absPath: "/working/dir/path", + safe: true, + }, + { + name: "illegal abs path", + root: "/working/dir", + absPath: "/working/dir/../but/not/really", + safe: false, + wantErr: "symlink traverses outside root boundary", + }, + { + name: "illegal root", + root: "working/dir/", + absPath: "/working/dir", + safe: false, + wantErr: "cannot calculate path relative to root for resolved symlink", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := isSecureSymlinkPath(tt.root, tt.absPath) + g.Expect(got).To(Equal(tt.safe)) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} diff --git a/internal/helm/chart/secureloader/file.go b/internal/helm/chart/secureloader/file.go new file mode 100644 index 00000000..ce42e4ed --- /dev/null +++ b/internal/helm/chart/secureloader/file.go @@ -0,0 +1,47 @@ +/* +Copyright The Helm Authors. +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package secureloader + +import ( + "io" + + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" +) + +// FileLoader is equal to Helm's. +// Redeclared to avoid having to deal with multiple package imports, +// possibly resulting in using the non-secure directory loader. +type FileLoader = loader.FileLoader + +// LoadFile loads from an archive file. +func LoadFile(name string) (*chart.Chart, error) { + return loader.LoadFile(name) +} + +// LoadArchiveFiles reads in files out of an archive into memory. This function +// performs important path security checks and should always be used before +// expanding a tarball +func LoadArchiveFiles(in io.Reader) ([]*loader.BufferedFile, error) { + return loader.LoadArchiveFiles(in) +} + +// LoadArchive loads from a reader containing a compressed tar archive. +func LoadArchive(in io.Reader) (*chart.Chart, error) { + return loader.LoadArchive(in) +} diff --git a/internal/helm/chart/loader/ignore/doc.go b/internal/helm/chart/secureloader/ignore/doc.go similarity index 100% rename from internal/helm/chart/loader/ignore/doc.go rename to internal/helm/chart/secureloader/ignore/doc.go diff --git a/internal/helm/chart/loader/ignore/rules.go b/internal/helm/chart/secureloader/ignore/rules.go similarity index 100% rename from internal/helm/chart/loader/ignore/rules.go rename to internal/helm/chart/secureloader/ignore/rules.go diff --git a/internal/helm/chart/loader/ignore/rules_test.go b/internal/helm/chart/secureloader/ignore/rules_test.go similarity index 100% rename from internal/helm/chart/loader/ignore/rules_test.go rename to internal/helm/chart/secureloader/ignore/rules_test.go diff --git a/internal/helm/chart/loader/ignore/testdata/.helmignore b/internal/helm/chart/secureloader/ignore/testdata/.helmignore similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/.helmignore rename to internal/helm/chart/secureloader/ignore/testdata/.helmignore diff --git a/internal/helm/chart/loader/ignore/testdata/.joonix b/internal/helm/chart/secureloader/ignore/testdata/.joonix similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/.joonix rename to internal/helm/chart/secureloader/ignore/testdata/.joonix diff --git a/internal/helm/chart/loader/ignore/testdata/a.txt b/internal/helm/chart/secureloader/ignore/testdata/a.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/a.txt rename to internal/helm/chart/secureloader/ignore/testdata/a.txt diff --git a/internal/helm/chart/loader/ignore/testdata/cargo/a.txt b/internal/helm/chart/secureloader/ignore/testdata/cargo/a.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/cargo/a.txt rename to internal/helm/chart/secureloader/ignore/testdata/cargo/a.txt diff --git a/internal/helm/chart/loader/ignore/testdata/cargo/b.txt b/internal/helm/chart/secureloader/ignore/testdata/cargo/b.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/cargo/b.txt rename to internal/helm/chart/secureloader/ignore/testdata/cargo/b.txt diff --git a/internal/helm/chart/loader/ignore/testdata/cargo/c.txt b/internal/helm/chart/secureloader/ignore/testdata/cargo/c.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/cargo/c.txt rename to internal/helm/chart/secureloader/ignore/testdata/cargo/c.txt diff --git a/internal/helm/chart/loader/ignore/testdata/helm.txt b/internal/helm/chart/secureloader/ignore/testdata/helm.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/helm.txt rename to internal/helm/chart/secureloader/ignore/testdata/helm.txt diff --git a/internal/helm/chart/loader/ignore/testdata/mast/a.txt b/internal/helm/chart/secureloader/ignore/testdata/mast/a.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/mast/a.txt rename to internal/helm/chart/secureloader/ignore/testdata/mast/a.txt diff --git a/internal/helm/chart/loader/ignore/testdata/mast/b.txt b/internal/helm/chart/secureloader/ignore/testdata/mast/b.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/mast/b.txt rename to internal/helm/chart/secureloader/ignore/testdata/mast/b.txt diff --git a/internal/helm/chart/loader/ignore/testdata/mast/c.txt b/internal/helm/chart/secureloader/ignore/testdata/mast/c.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/mast/c.txt rename to internal/helm/chart/secureloader/ignore/testdata/mast/c.txt diff --git a/internal/helm/chart/loader/ignore/testdata/rudder.txt b/internal/helm/chart/secureloader/ignore/testdata/rudder.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/rudder.txt rename to internal/helm/chart/secureloader/ignore/testdata/rudder.txt diff --git a/internal/helm/chart/loader/ignore/testdata/templates/.dotfile b/internal/helm/chart/secureloader/ignore/testdata/templates/.dotfile similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/templates/.dotfile rename to internal/helm/chart/secureloader/ignore/testdata/templates/.dotfile diff --git a/internal/helm/chart/loader/ignore/testdata/tiller.txt b/internal/helm/chart/secureloader/ignore/testdata/tiller.txt similarity index 100% rename from internal/helm/chart/loader/ignore/testdata/tiller.txt rename to internal/helm/chart/secureloader/ignore/testdata/tiller.txt diff --git a/internal/helm/chart/secureloader/loader.go b/internal/helm/chart/secureloader/loader.go new file mode 100644 index 00000000..86ff2cf6 --- /dev/null +++ b/internal/helm/chart/secureloader/loader.go @@ -0,0 +1,76 @@ +/* +Copyright The Helm Authors. +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package secureloader + +import ( + "errors" + "io/fs" + "os" + "path/filepath" + + securejoin "github.com/cyphar/filepath-securejoin" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" +) + +// Loader returns a new loader.ChartLoader appropriate for the given chart +// name. That being, SecureDirLoader when name is a directory, and +// FileLoader when it's a file. +// Name can be an absolute or relative path, but always has to be inside +// root. +func Loader(root, name string) (loader.ChartLoader, error) { + root, name = filepath.Clean(root), filepath.Clean(name) + relName := name + if filepath.IsAbs(relName) { + var err error + if relName, err = filepath.Rel(root, name); err != nil { + return nil, err + } + } + secureName, err := securejoin.SecureJoin(root, relName) + if err != nil { + return nil, err + } + fi, err := os.Lstat(secureName) + if err != nil { + if pathErr := new(fs.PathError); errors.As(err, &pathErr) { + return nil, &fs.PathError{Op: pathErr.Op, Path: name, Err: pathErr.Err} + } + return nil, err + } + if fi.IsDir() { + return NewSecureDirLoader(root, secureName, 0), nil + } + return FileLoader(secureName), nil +} + +// Load takes a string root and name, tries to resolve it to a file or directory, +// and then loads it securely without traversing outside of root. +// +// This is the preferred way to load a chart. It will discover the chart encoding +// and hand off to the appropriate chart reader. +// +// If a .helmignore file is present, the directory loader will skip loading any files +// matching it. But .helmignore is not evaluated when reading out of an archive. +func Load(root, name string) (*chart.Chart, error) { + l, err := Loader(root, name) + if err != nil { + return nil, err + } + return l.Load() +} diff --git a/internal/helm/chart/secureloader/loader_test.go b/internal/helm/chart/secureloader/loader_test.go new file mode 100644 index 00000000..d0b69a84 --- /dev/null +++ b/internal/helm/chart/secureloader/loader_test.go @@ -0,0 +1,54 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package secureloader + +import ( + "io/fs" + "os" + "path/filepath" + "testing" + + . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/chart/loader" +) + +func TestLoader(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + fakeChart := filepath.Join(tmpDir, "fake.tgz") + g.Expect(os.WriteFile(fakeChart, []byte(""), 0o644)).To(Succeed()) + + got, err := Loader(tmpDir, fakeChart) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(loader.FileLoader(fakeChart))) + + fakeChartPath := filepath.Join(tmpDir, "fake") + g.Expect(os.Mkdir(fakeChartPath, 0o700)).To(Succeed()) + got, err = Loader(tmpDir, "fake") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(SecureDirLoader{root: tmpDir, dir: fakeChartPath, maxSize: DefaultMaxFileSize})) + + symlinkRoot := filepath.Join(tmpDir, "symlink") + g.Expect(os.Mkdir(symlinkRoot, 0o700)).To(Succeed()) + symlinkPath := filepath.Join(symlinkRoot, "fake.tgz") + g.Expect(os.Symlink(fakeChart, symlinkPath)) + got, err = Loader(symlinkRoot, symlinkPath) + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(BeAssignableToTypeOf(&fs.PathError{})) + g.Expect(got).To(BeNil()) +} diff --git a/internal/helm/chart/loader/sympath/walk.go b/internal/helm/chart/secureloader/sympath/walk.go similarity index 100% rename from internal/helm/chart/loader/sympath/walk.go rename to internal/helm/chart/secureloader/sympath/walk.go diff --git a/internal/helm/chart/loader/sympath/walk_test.go b/internal/helm/chart/secureloader/sympath/walk_test.go similarity index 100% rename from internal/helm/chart/loader/sympath/walk_test.go rename to internal/helm/chart/secureloader/sympath/walk_test.go