diff --git a/internal/blueprint/filesystem_customizations.go b/internal/blueprint/filesystem_customizations.go index d998f1ff1..02bc36439 100644 --- a/internal/blueprint/filesystem_customizations.go +++ b/internal/blueprint/filesystem_customizations.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/osbuild/osbuild-composer/internal/common" - "github.com/osbuild/osbuild-composer/internal/pathpolicy" ) type FilesystemCustomization struct { @@ -70,20 +69,3 @@ func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error { return nil } - -// CheckMountpointsPolicy checks if the mountpoints are allowed by the policy -func CheckMountpointsPolicy(mountpoints []FilesystemCustomization, mountpointAllowList *pathpolicy.PathPolicies) error { - invalidMountpoints := []string{} - for _, m := range mountpoints { - err := mountpointAllowList.Check(m.Mountpoint) - if err != nil { - invalidMountpoints = append(invalidMountpoints, m.Mountpoint) - } - } - - if len(invalidMountpoints) > 0 { - return fmt.Errorf("The following custom mountpoints are not supported %+q", invalidMountpoints) - } - - return nil -} diff --git a/internal/blueprint/fsnode_customizations.go b/internal/blueprint/fsnode_customizations.go index cce499641..92ba2f676 100644 --- a/internal/blueprint/fsnode_customizations.go +++ b/internal/blueprint/fsnode_customizations.go @@ -12,7 +12,6 @@ import ( "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/fsnode" - "github.com/osbuild/osbuild-composer/internal/pathpolicy" ) // validateModeString checks that the given string is a valid mode octal number @@ -435,37 +434,3 @@ func ValidateDirFileCustomizations(dirs []DirectoryCustomization, files []FileCu return nil } - -// CheckFileCustomizationsPolicy checks if the given File customizations are allowed by the path policy. -// If any of the customizations are not allowed by the path policy, an error is returned. Otherwise, nil is returned. -func CheckFileCustomizationsPolicy(files []FileCustomization, pathPolicy *pathpolicy.PathPolicies) error { - var invalidPaths []string - for _, file := range files { - if err := pathPolicy.Check(file.Path); err != nil { - invalidPaths = append(invalidPaths, file.Path) - } - } - - if len(invalidPaths) > 0 { - return fmt.Errorf("the following custom files are not allowed: %+q", invalidPaths) - } - - return nil -} - -// CheckDirectoryCustomizationsPolicy checks if the given Directory customizations are allowed by the path policy. -// If any of the customizations are not allowed by the path policy, an error is returned. Otherwise, nil is returned. -func CheckDirectoryCustomizationsPolicy(dirs []DirectoryCustomization, pathPolicy *pathpolicy.PathPolicies) error { - var invalidPaths []string - for _, dir := range dirs { - if err := pathPolicy.Check(dir.Path); err != nil { - invalidPaths = append(invalidPaths, dir.Path) - } - } - - if len(invalidPaths) > 0 { - return fmt.Errorf("the following custom directories are not allowed: %+q", invalidPaths) - } - - return nil -} diff --git a/internal/blueprint/fsnode_customizations_test.go b/internal/blueprint/fsnode_customizations_test.go index 22d66023d..1f25730ab 100644 --- a/internal/blueprint/fsnode_customizations_test.go +++ b/internal/blueprint/fsnode_customizations_test.go @@ -8,7 +8,6 @@ import ( "github.com/BurntSushi/toml" "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/fsnode" - "github.com/osbuild/osbuild-composer/internal/pathpolicy" "github.com/stretchr/testify/assert" ) @@ -1133,122 +1132,3 @@ func TestValidateDirFileCustomizations(t *testing.T) { }) } } - -func TestCheckFileCustomizationsPolicy(t *testing.T) { - policy := map[string]pathpolicy.PathPolicy{ - "/": {Deny: true}, - "/etc": {}, - "/etc/fstab": {Deny: true}, - "/etc/os-release": {Deny: true}, - "/etc/hostname": {Deny: true}, - "/etc/shadow": {Deny: true}, - "/etc/passwd": {Deny: true}, - "/etc/group": {Deny: true}, - } - pathPolicy := pathpolicy.NewPathPolicies(policy) - - testCases := []struct { - Name string - Files []FileCustomization - Error bool - }{ - { - Name: "disallowed-file", - Files: []FileCustomization{ - { - Path: "/etc/shadow", - }, - }, - Error: true, - }, - { - Name: "disallowed-file-2", - Files: []FileCustomization{ - { - Path: "/home/user/.ssh/authorized_keys", - }, - }, - Error: true, - }, - { - Name: "disallowed-file-3", - Files: []FileCustomization{ - { - Path: "/file", - }, - }, - Error: true, - }, - { - Name: "allowed-file-named", - Files: []FileCustomization{ - { - Path: "/etc/named.conf", - }, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.Name, func(t *testing.T) { - err := CheckFileCustomizationsPolicy(tc.Files, pathPolicy) - if tc.Error { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - -func TestCheckDirectoryCustomizationsPolicy(t *testing.T) { - policy := map[string]pathpolicy.PathPolicy{ - "/": {Deny: true}, - "/etc": {}, - } - pathPolicy := pathpolicy.NewPathPolicies(policy) - - testCases := []struct { - Name string - Directories []DirectoryCustomization - Error bool - }{ - { - Name: "disallowed-directory", - Directories: []DirectoryCustomization{ - { - Path: "/dir", - }, - }, - Error: true, - }, - { - Name: "disallowed-directory-2", - Directories: []DirectoryCustomization{ - { - Path: "/var/log/fancy-dir", - }, - }, - Error: true, - }, - { - Name: "allowed-directory", - Directories: []DirectoryCustomization{ - { - Path: "/etc/systemd/system/sshd.service.d", - }, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.Name, func(t *testing.T) { - err := CheckDirectoryCustomizationsPolicy(tc.Directories, pathPolicy) - if tc.Error { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} diff --git a/internal/pathpolicy/path_policy.go b/internal/pathpolicy/path_policy.go deleted file mode 100644 index 423a5751b..000000000 --- a/internal/pathpolicy/path_policy.go +++ /dev/null @@ -1,54 +0,0 @@ -package pathpolicy - -import ( - "fmt" - "path" -) - -type PathPolicy struct { - Deny bool // explicitly do not allow this entry - Exact bool // require and exact match, no subdirs -} - -type PathPolicies = PathTrie - -// Create a new PathPolicies trie from a map of path to PathPolicy -func NewPathPolicies(entries map[string]PathPolicy) *PathPolicies { - - noType := make(map[string]interface{}, len(entries)) - - for k, v := range entries { - noType[k] = v - } - - return NewPathTrieFromMap(noType) -} - -// Check a given path against the PathPolicies -func (pol *PathPolicies) Check(fsPath string) error { - - // Quickly check we have a path and it is absolute - if fsPath == "" || fsPath[0] != '/' { - return fmt.Errorf("path must be absolute") - } - - // ensure that only clean paths are valid - if fsPath != path.Clean(fsPath) { - return fmt.Errorf("path must be canonical") - } - - node, left := pol.Lookup(fsPath) - policy, ok := node.Payload.(PathPolicy) - if !ok { - panic("programming error: invalid path trie payload") - } - - // 1) path is explicitly not allowed or - // 2) a subpath was match but an explicit match is required - if policy.Deny || (policy.Exact && len(left) > 0) { - return fmt.Errorf("path '%s ' is not allowed", fsPath) - } - - // exact match or recursive path allowed - return nil -} diff --git a/internal/pathpolicy/path_policy_test.go b/internal/pathpolicy/path_policy_test.go deleted file mode 100644 index 28dec75d1..000000000 --- a/internal/pathpolicy/path_policy_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package pathpolicy - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestPathPolicyCheck(t *testing.T) { - assert := assert.New(t) - - entires := map[string]PathPolicy{ - "/": {Exact: true}, - "/boot": {Exact: true}, - "/boot/efi": {Exact: true}, - "/var": {}, - "/var/empty": {Deny: true}, - "/srv": {}, - "/home": {}, - } - - policies := NewPathPolicies(entires) - assert.NotNil(policies) - - tests := map[string]bool{ - "/": true, - "/custom": false, - "/boot": true, - "/boot/grub2": false, - "/boot/efi": true, - "/boot/efi/redora": false, - "/srv": true, - "/srv/www": true, - "/srv/www/data": true, - "/var": true, - "/var/log": true, - "/var/empty": false, - "/var/empty/dir": false, - } - - for k, v := range tests { - err := policies.Check(k) - if v { - assert.NoError(err) - } else { - assert.Errorf(err, "unexpected error for path '%s'", k) - } - } -} diff --git a/internal/pathpolicy/path_tree_test.go b/internal/pathpolicy/path_tree_test.go deleted file mode 100644 index 71c195d6d..000000000 --- a/internal/pathpolicy/path_tree_test.go +++ /dev/null @@ -1,135 +0,0 @@ -package pathpolicy - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/osbuild/osbuild-composer/internal/common" -) - -func TestNewPathTrieFromMap(t *testing.T) { - assert := assert.New(t) - - type testCase struct { - entries map[string]interface{} - trie *PathTrie - } - - tests := []testCase{ - { - entries: map[string]interface{}{}, - trie: &PathTrie{ - Name: []string{}, - }, - }, - { - entries: map[string]interface{}{ - "/": common.ToPtr(1), - }, - trie: &PathTrie{ - Name: []string{}, - Payload: common.ToPtr(1), - }, - }, - { - entries: map[string]interface{}{ - "/": common.ToPtr(1), - "/var": common.ToPtr(2), - "/var/lib/chrony": common.ToPtr(3), - "/var/lib/chrony/logs": common.ToPtr(4), - "/var/lib/osbuild": common.ToPtr(5), - "/var/lib/osbuild/store/cache": common.ToPtr(6), - "/boot": common.ToPtr(7), - "/boot/efi": common.ToPtr(8), - }, - trie: &PathTrie{ - Name: []string{}, - Payload: common.ToPtr(1), - Paths: []*PathTrie{ - { - Name: []string{"boot"}, - Payload: common.ToPtr(7), - Paths: []*PathTrie{ - { - Name: []string{"efi"}, - Payload: common.ToPtr(8), - }, - }, - }, - { - Name: []string{"var"}, - Payload: common.ToPtr(2), - Paths: []*PathTrie{ - { - Name: []string{"lib", "chrony"}, - Payload: common.ToPtr(3), - Paths: []*PathTrie{ - { - Name: []string{"logs"}, - Payload: common.ToPtr(4), - }, - }, - }, - { - Name: []string{"lib", "osbuild"}, - Payload: common.ToPtr(5), - Paths: []*PathTrie{ - { - Name: []string{"store", "cache"}, - Payload: common.ToPtr(6), - }, - }, - }, - }, - }, - }, - }, - }, - } - - for _, tc := range tests { - have := NewPathTrieFromMap(tc.entries) - assert.NotNil(have) - assert.Equal(tc.trie, have) - } -} - -func TestPathTrieLookup(t *testing.T) { - assert := assert.New(t) - - entries := map[string]interface{}{ - "/": "/", - "/boot": "/boot", - "/boot/efi": "/boot/efi", - "/var": "/var", - "/var/lib/osbuild": "/var/lib/osbuild", - "/var/lib/osbuild/store/cache": "/var/lib/osbuild/store/cache", - "/var/lib/chrony": "/var/lib/chrony", - "/var/lib/chrony/logs": "/var/lib/chrony/logs", - } - - trie := NewPathTrieFromMap(entries) - - testCases := map[string]string{ - "/": "/", - "/srv": "/", - "/srv/data": "/", - "/boot": "/boot", - "/boot/efi": "/boot/efi", - "/boot/grub2": "/boot", - "/boot/efi/fedora": "/boot/efi", - "/var/lib/osbuild": "/var/lib/osbuild", - "/var/lib/osbuild/test": "/var/lib/osbuild", - "/var/lib/chrony": "/var/lib/chrony", - "/var/lib/chrony/test": "/var/lib/chrony", - "/var/lib/chrony/logs": "/var/lib/chrony/logs", - "/var/lib/chrony/logs/data": "/var/lib/chrony/logs", - } - - for k, v := range testCases { - node, _ := trie.Lookup(k) - assert.NotNil(node) - assert.Equal(v, node.Payload, "Lookup path: '%s' (%+v)", k, node.Name) - } -} diff --git a/internal/pathpolicy/path_trie.go b/internal/pathpolicy/path_trie.go deleted file mode 100644 index 1de1f46a6..000000000 --- a/internal/pathpolicy/path_trie.go +++ /dev/null @@ -1,123 +0,0 @@ -package pathpolicy - -import ( - "sort" - "strings" -) - -// splits the path into its individual components. Retruns the -// empty list if the path is just the absolute root, i.e. "/". -func pathTrieSplitPath(path string) []string { - path = strings.Trim(path, "/") - if path == "" { - return []string{} - } - - return strings.Split(path, "/") -} - -type PathTrie struct { - Name []string - Paths []*PathTrie - Payload interface{} -} - -// match checks if the given trie is a prefix of path -func (trie *PathTrie) match(path []string) bool { - if len(trie.Name) > len(path) { - return false - } - - for i := range trie.Name { - if path[i] != trie.Name[i] { - return false - } - } - - return true -} - -func (trie *PathTrie) get(path []string) (*PathTrie, []string) { - if len(path) < 1 { - panic("programming error: expected root node") - } - - var node *PathTrie - for i := range trie.Paths { - if trie.Paths[i].match(path) { - node = trie.Paths[i] - break - } - } - - // no subpath match, we are the best match - if node == nil { - return trie, path - } - - // node, or one of its sub-nodes, is a match - prefix := len(node.Name) - - // the node is a perfect match, return it - if len(path) == prefix { - return node, nil - } - - // check if any sub-path's of node match - return node.get(path[prefix:]) -} - -func (trie *PathTrie) add(path []string) *PathTrie { - node := &PathTrie{Name: path} - - if trie.Paths == nil { - trie.Paths = make([]*PathTrie, 0, 1) - } - - trie.Paths = append(trie.Paths, node) - - return node -} - -// Construct a new trie from a map of paths to their payloads. -// Returns the root node of the trie. -func NewPathTrieFromMap(entries map[string]interface{}) *PathTrie { - root := &PathTrie{Name: []string{}} - - keys := make([]string, 0, len(entries)) - for k := range entries { - keys = append(keys, k) - } - - sort.Strings(keys) - - for _, k := range keys { - node, left := root.Lookup(k) - - if len(left) > 0 { - node = node.add(left) - } - - node.Payload = entries[k] - } - - return root -} - -// Lookup returns the node that is the prefix of path and -// the unmatched path segment. Must be called on the root -// trie node. -func (root *PathTrie) Lookup(path string) (*PathTrie, []string) { - - if len(root.Name) != 0 { - panic("programming error: lookup on non-root trie node") - } - - elements := pathTrieSplitPath(path) - - if len(elements) == 0 { - return root, elements - } - - return root.get(elements) -} diff --git a/internal/pathpolicy/policies.go b/internal/pathpolicy/policies.go deleted file mode 100644 index aa4d315c9..000000000 --- a/internal/pathpolicy/policies.go +++ /dev/null @@ -1,31 +0,0 @@ -package pathpolicy - -// MountpointPolicies is a set of default mountpoint policies used for filesystem customizations -var MountpointPolicies = NewPathPolicies(map[string]PathPolicy{ - "/": {Exact: true}, - "/boot": {Exact: true}, - "/var": {}, - "/opt": {}, - "/srv": {}, - "/usr": {}, - "/app": {}, - "/data": {}, - "/home": {}, - "/tmp": {}, -}) - -// CustomDirectoriesPolicies is a set of default policies for custom directories -var CustomDirectoriesPolicies = NewPathPolicies(map[string]PathPolicy{ - "/": {Deny: true}, - "/etc": {}, -}) - -// CustomFilesPolicies is a set of default policies for custom files -var CustomFilesPolicies = NewPathPolicies(map[string]PathPolicy{ - "/": {Deny: true}, - "/etc": {}, - "/etc/fstab": {Deny: true}, - "/etc/shadow": {Deny: true}, - "/etc/passwd": {Deny: true}, - "/etc/group": {Deny: true}, -})