From b3eee178462ad0a467ee2c4b4df4d60039e97286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Pa=C3=9F?= <22845248+mpass99@users.noreply.github.com> Date: Tue, 25 Oct 2022 11:38:16 +0100 Subject: [PATCH] Support protected directories by setting the sticky bit to all explicitly requested directories. --- api/swagger.yaml | 2 +- internal/runner/nomad_runner.go | 2 +- tests/e2e/runners_test.go | 94 +++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index 04050d4..da31a87 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -404,7 +404,7 @@ paths: type: object properties: path: - description: Location where the file should be placed. Can be absolute (starting with /) or relative to the workspace directory. Missing parent directories are created. If this ends with a /, the path is interpreted as a directory and content is ignored + description: Location where the file should be placed. Can be absolute (starting with /) or relative to the workspace directory. Missing parent directories are created. If this ends with a /, the path is interpreted as a directory and content is ignored. Currently, every file/directory is owned by root but the directories have the sticky bit set to allow unprivileged file creation. type: string example: /etc/passwd content: diff --git a/internal/runner/nomad_runner.go b/internal/runner/nomad_runner.go index d569395..9fa7e8b 100644 --- a/internal/runner/nomad_runner.go +++ b/internal/runner/nomad_runner.go @@ -342,7 +342,7 @@ func tarHeader(file dto.File) *tar.Header { return &tar.Header{ Typeflag: tar.TypeDir, Name: file.CleanedPath(), - Mode: 0o755, + Mode: 0o1777, // See #236. Sticky bit is to allow creating files next to write-protected files. } } else { return &tar.Header{ diff --git a/tests/e2e/runners_test.go b/tests/e2e/runners_test.go index 36e59a0..4ef817b 100644 --- a/tests/e2e/runners_test.go +++ b/tests/e2e/runners_test.go @@ -325,6 +325,100 @@ func (s *E2ETestSuite) TestCopyFilesRoute_PermissionDenied() { }) } +func (s *E2ETestSuite) TestCopyFilesRoute_ProtectedFolders() { + runnerID, err := ProvideRunner(&dto.RunnerRequest{ExecutionEnvironmentID: tests.DefaultEnvironmentIDAsInteger}) + s.NoError(err) + + // Initialization of protected folder + newFileContent := []byte("New content") + protectedFolderPath := dto.FilePath("protectedFolder/") + copyFilesRequestByteString, err := json.Marshal(&dto.UpdateFileSystemRequest{ + Copy: []dto.File{ + {Path: protectedFolderPath}, + {Path: protectedFolderPath + tests.DefaultFileName, Content: newFileContent}, + }, + }) + s.Require().NoError(err) + + resp, err := helpers.HTTPPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID, api.UpdateFileSystemPath), + "application/json", bytes.NewReader(copyFilesRequestByteString)) + s.NoError(err) + s.Equal(http.StatusNoContent, resp.StatusCode) + + // User manipulates protected folder + s.Run("User can create files", func() { + webSocketURL, err := ProvideWebSocketURL(&s.Suite, runnerID, &dto.ExecutionRequest{ + Command: fmt.Sprintf("touch %s/userfile", protectedFolderPath), + TimeLimit: int(tests.DefaultTestTimeout.Seconds()), + PrivilegedExecution: false, + }) + s.Require().NoError(err) + connection, err := ConnectToWebSocket(webSocketURL) + s.Require().NoError(err) + messages, err := helpers.ReceiveAllWebSocketMessages(connection) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + stdout, stderr, _ := helpers.WebSocketOutputMessages(messages) + s.Empty(stdout) + s.Empty(stderr) + }) + + s.Run("User can not delete protected folder", func() { + webSocketURL, err := ProvideWebSocketURL(&s.Suite, runnerID, &dto.ExecutionRequest{ + Command: fmt.Sprintf("rm -fr %s", protectedFolderPath), + TimeLimit: int(tests.DefaultTestTimeout.Seconds()), + PrivilegedExecution: false, + }) + s.Require().NoError(err) + connection, err := ConnectToWebSocket(webSocketURL) + s.Require().NoError(err) + messages, err := helpers.ReceiveAllWebSocketMessages(connection) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + stdout, stderr, _ := helpers.WebSocketOutputMessages(messages) + s.Empty(stdout) + s.Contains(stderr, "Operation not permitted") + }) + + s.Run("User can not delete protected file", func() { + webSocketURL, err := ProvideWebSocketURL(&s.Suite, runnerID, &dto.ExecutionRequest{ + Command: fmt.Sprintf("rm -f %s", protectedFolderPath+tests.DefaultFileName), + TimeLimit: int(tests.DefaultTestTimeout.Seconds()), + PrivilegedExecution: false, + }) + s.Require().NoError(err) + connection, err := ConnectToWebSocket(webSocketURL) + s.Require().NoError(err) + messages, err := helpers.ReceiveAllWebSocketMessages(connection) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + stdout, stderr, _ := helpers.WebSocketOutputMessages(messages) + s.Empty(stdout) + s.Contains(stderr, "Operation not permitted") + }) + + s.Run("User can not write protected file", func() { + webSocketURL, err := ProvideWebSocketURL(&s.Suite, runnerID, &dto.ExecutionRequest{ + Command: fmt.Sprintf("echo Hi >> %s", protectedFolderPath+tests.DefaultFileName), + TimeLimit: int(tests.DefaultTestTimeout.Seconds()), + PrivilegedExecution: false, + }) + s.Require().NoError(err) + connection, err := ConnectToWebSocket(webSocketURL) + s.Require().NoError(err) + messages, err := helpers.ReceiveAllWebSocketMessages(connection) + s.Require().Error(err) + s.Equal(&websocket.CloseError{Code: websocket.CloseNormalClosure}, err) + stdout, stderr, _ := helpers.WebSocketOutputMessages(messages) + s.Empty(stdout) + s.Contains(stderr, "Permission denied") + }) + + s.Run("File content is not manipulated", func() { + s.assertFileContent(runnerID, string(protectedFolderPath+tests.DefaultFileName), string(newFileContent)) + }) +} + func (s *E2ETestSuite) TestGetFileContent_Nomad() { runnerID, err := ProvideRunner(&dto.RunnerRequest{ ExecutionEnvironmentID: tests.DefaultEnvironmentIDAsInteger,