From 86976a51beac39f9ecd7df7e4e2bfdc10973b980 Mon Sep 17 00:00:00 2001 From: Shash Reddy Date: Tue, 6 Aug 2019 13:34:49 -0700 Subject: [PATCH 1/4] Add handler with file access info - Use permissions to determine whether we can read or write to file/directory. - Validate response body for file access information Signed-off-by: Shash Reddy --- test/conformance/runtime/filesystem_test.go | 8 ++ .../runtime/handlers/file_access_attempt.go | 107 ++++++++++++++++++ test/test_images/runtime/handlers/runtime.go | 34 +++++- test/types/runtime.go | 12 ++ 4 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 test/test_images/runtime/handlers/file_access_attempt.go diff --git a/test/conformance/runtime/filesystem_test.go b/test/conformance/runtime/filesystem_test.go index 5c34ee7cd162..beeb789f7840 100644 --- a/test/conformance/runtime/filesystem_test.go +++ b/test/conformance/runtime/filesystem_test.go @@ -72,6 +72,14 @@ func testFiles(t *testing.T, clients *test.Clients, paths map[string]types.FileI return fmt.Errorf("%s has invalid permissions string %s: %w", path, riFile.Perm, err) } } + + riFileAccess, ok := ri.Host.FileAccess[path] + if ok && riFileAccess.ReadErr != "" { + return fmt.Errorf("Want no read errors for file %s, got error: %s", path, riFileAccess.ReadErr) + } + if ok && riFileAccess.WriteErr != "" { + return fmt.Errorf("Want no write errors for file %s, got error: %s", path, riFileAccess.WriteErr) + } } return nil } diff --git a/test/test_images/runtime/handlers/file_access_attempt.go b/test/test_images/runtime/handlers/file_access_attempt.go new file mode 100644 index 000000000000..34bd3a09775d --- /dev/null +++ b/test/test_images/runtime/handlers/file_access_attempt.go @@ -0,0 +1,107 @@ +/* +Copyright 2019 The Knative 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 handlers + +import ( + "io/ioutil" + "os" + + "knative.dev/serving/test/types" +) + +type permissionBits uint32 + +const ( + userRead permissionBits = 1 << (9 - 1 - iota) + userWrite + userExecute + groupRead + groupWrite + groupExecute + otherRead + otherWrite + otherExecute +) + +func (p permissionBits) hasPermission(mode permissionBits) bool { + return p&mode == mode +} + +func fileAccessAttempt(filePaths ...string) map[string]types.FileAccessInfo { + files := map[string]types.FileAccessInfo{} + for _, path := range filePaths { + accessInfo := types.FileAccessInfo{} + + if err := checkReadable(path); err != nil { + accessInfo.ReadErr = err.Error() + } + + if err := checkWritable(path); err != nil { + accessInfo.WriteErr = err.Error() + } + + files[path] = accessInfo + } + return files +} + +// checkReadable function checks whether path file or directory is readable +func checkReadable(path string) error { + file, err := os.Stat(path) // It should only return error only if file does not exist + if err != nil { + return err + } + + // We aren't expected to be able to read, so just exit + perm := permissionBits(file.Mode().Perm()) + if !perm.hasPermission(otherRead) { + return nil + } + + if file.IsDir() { + _, err := ioutil.ReadDir(path) + return err + } + readFile, err := os.OpenFile(path, os.O_RDONLY, 0) + defer readFile.Close() + return err +} + +// checkWritable function checks whether path file or directory is writable +func checkWritable(path string) error { + file, err := os.Stat(path) // It should only return error only if file does not exist + if err != nil { + return err + } + + // We aren't expected to be able to write, so just exits + perm := permissionBits(file.Mode().Perm()) + if !perm.hasPermission(otherWrite) { + return nil + } + + if file.IsDir() { + writeFile, err := ioutil.TempFile(path, "random") + if writeFile != nil { + os.Remove(writeFile.Name()) + } + return err + } + + writeFile, err := os.OpenFile(path, os.O_APPEND, 0) + if writeFile != nil { + defer writeFile.Close() + } + return err +} diff --git a/test/test_images/runtime/handlers/runtime.go b/test/test_images/runtime/handlers/runtime.go index 85d06ce51cb4..74549315781e 100644 --- a/test/test_images/runtime/handlers/runtime.go +++ b/test/test_images/runtime/handlers/runtime.go @@ -20,6 +20,11 @@ import ( "knative.dev/serving/test/types" ) +var fileAccessExclusions = []string{ + "/dev/tty", + "/dev/console", +} + func runtimeHandler(w http.ResponseWriter, r *http.Request) { log.Println("Retrieving Runtime Information") w.Header().Set("Content-Type", "application/json") @@ -38,14 +43,31 @@ func runtimeHandler(w http.ResponseWriter, r *http.Request) { k := &types.RuntimeInfo{ Request: requestInfo(r), Host: &types.HostInfo{EnvVars: env(), - Files: fileInfo(filePaths...), - Cgroups: cgroups(cgroupPaths...), - Mounts: mounts(), - Stdin: stdin(), - User: userInfo(), - Args: args(), + Files: fileInfo(filePaths...), + FileAccess: fileAccessAttempt(excludeFilePaths(filePaths, fileAccessExclusions)...), + Cgroups: cgroups(cgroupPaths...), + Mounts: mounts(), + Stdin: stdin(), + User: userInfo(), + Args: args(), }, } writeJSON(w, k) } + +func excludeFilePaths(filePaths []string, excludedPaths []string) []string { + var paths []string + for _, path := range filePaths { + excluded := false + for i := len(excludedPaths) - 1; i >= 0 && !excluded; i-- { + excluded = path == excludedPaths[i] + } + + if !excluded { + paths = append(paths, path) + } + } + + return paths +} diff --git a/test/types/runtime.go b/test/types/runtime.go index 08744adfb20d..c8dd7c242e18 100644 --- a/test/types/runtime.go +++ b/test/types/runtime.go @@ -141,6 +141,8 @@ type HostInfo struct { Files map[string]FileInfo `json:"files"` // EnvVars is a map of all environment variables set. EnvVars map[string]string `json:"envs"` + // FileAccess is a map of file access information + FileAccess map[string]FileAccessInfo `json:"fileaccess"` // Cgroups is a list of cgroup information. Cgroups []*Cgroup `json:"cgroups"` // Mounts is a list of mounted volume information, or error. @@ -189,6 +191,16 @@ type FileInfo struct { Error string `json:"error,omitempty"` } +// FileAccessInfo contains the file access information +type FileAccessInfo struct { + // ReadErr is the String representation of an error received when attempting to read + // a file or directory + ReadErr string `json:"read_error,omitempty"` + // WriteErr is the String representation of an error received when attempting to write + // to a file or directory + WriteErr string `json:"write_error,omitempty"` +} + // Cgroup contains the Cgroup value for a given setting. type Cgroup struct { // Name is the full path name of the cgroup. From 76f8c98866e760222eedd8a36195aea402ff07e2 Mon Sep 17 00:00:00 2001 From: Andrew Su Date: Mon, 30 Sep 2019 11:20:25 -0400 Subject: [PATCH 2/4] Address comments --- .../runtime/handlers/file_access_attempt.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/test/test_images/runtime/handlers/file_access_attempt.go b/test/test_images/runtime/handlers/file_access_attempt.go index 34bd3a09775d..bc16e9933a7b 100644 --- a/test/test_images/runtime/handlers/file_access_attempt.go +++ b/test/test_images/runtime/handlers/file_access_attempt.go @@ -1,10 +1,13 @@ /* Copyright 2019 The Knative Authors - Licensed under the Apache License, Version 2.0 (the "License"); + +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 + + 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 @@ -23,15 +26,8 @@ import ( type permissionBits uint32 const ( - userRead permissionBits = 1 << (9 - 1 - iota) - userWrite - userExecute - groupRead - groupWrite - groupExecute - otherRead + otherRead permissionBits = 1 << (2 - iota) otherWrite - otherExecute ) func (p permissionBits) hasPermission(mode permissionBits) bool { From f3c96a4004aabe5485c9a7b8e89d3a4205b91204 Mon Sep 17 00:00:00 2001 From: Shash Reddy Date: Mon, 28 Oct 2019 11:18:30 -0700 Subject: [PATCH 3/4] Address comments - Thanks to Markus for suggestion about simplify the logic to get excluded file paths --- test/test_images/runtime/handlers/file_access_attempt.go | 7 +++++-- test/test_images/runtime/handlers/runtime.go | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/test/test_images/runtime/handlers/file_access_attempt.go b/test/test_images/runtime/handlers/file_access_attempt.go index bc16e9933a7b..8549cc7126cb 100644 --- a/test/test_images/runtime/handlers/file_access_attempt.go +++ b/test/test_images/runtime/handlers/file_access_attempt.go @@ -70,8 +70,11 @@ func checkReadable(path string) error { return err } readFile, err := os.OpenFile(path, os.O_RDONLY, 0) - defer readFile.Close() - return err + if err != nil { + return err + } + readFile.Close() + return nil } // checkWritable function checks whether path file or directory is writable diff --git a/test/test_images/runtime/handlers/runtime.go b/test/test_images/runtime/handlers/runtime.go index 74549315781e..21d29acf5b36 100644 --- a/test/test_images/runtime/handlers/runtime.go +++ b/test/test_images/runtime/handlers/runtime.go @@ -60,14 +60,16 @@ func excludeFilePaths(filePaths []string, excludedPaths []string) []string { var paths []string for _, path := range filePaths { excluded := false - for i := len(excludedPaths) - 1; i >= 0 && !excluded; i-- { - excluded = path == excludedPaths[i] + for _, excludedPath := range excludedPaths { + if path == excludedPath { + excluded = true + break + } } if !excluded { paths = append(paths, path) } } - return paths } From 49f5e5791a768e1736cda1841c444e3830890c35 Mon Sep 17 00:00:00 2001 From: Shash Reddy Date: Tue, 29 Oct 2019 09:41:49 -0700 Subject: [PATCH 4/4] Address comment --- test/test_images/runtime/handlers/file_access_attempt.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_images/runtime/handlers/file_access_attempt.go b/test/test_images/runtime/handlers/file_access_attempt.go index 8549cc7126cb..92ac3a20e7df 100644 --- a/test/test_images/runtime/handlers/file_access_attempt.go +++ b/test/test_images/runtime/handlers/file_access_attempt.go @@ -69,12 +69,12 @@ func checkReadable(path string) error { _, err := ioutil.ReadDir(path) return err } + readFile, err := os.OpenFile(path, os.O_RDONLY, 0) if err != nil { return err } - readFile.Close() - return nil + return readFile.Close() } // checkWritable function checks whether path file or directory is writable