diff --git a/oss-internship-2020/sapi_libarchive/examples/sandbox.h b/oss-internship-2020/sapi_libarchive/examples/sandbox.h index b0ae390..c3b6c5a 100644 --- a/oss-internship-2020/sapi_libarchive/examples/sandbox.h +++ b/oss-internship-2020/sapi_libarchive/examples/sandbox.h @@ -7,9 +7,11 @@ #include "helpers.h" #include "libarchive_sapi.sapi.h" +// When creating an archive, we need read permissions on each of the file/directory added +// in the archive. Also, in order to create the archive, we map /output with the basename of +// the archive. This way, the program can create the file without having access to anything else. class SapiLibarchiveSandboxCreate : public LibarchiveSandbox { public: - // TODO explicit SapiLibarchiveSandboxCreate(const std::vector& files, absl::string_view archive_path) : files_(files), archive_path_(archive_path) {} @@ -46,8 +48,8 @@ class SapiLibarchiveSandboxCreate : public LibarchiveSandbox { __NR_getdents64, }); + // Here we only check whether the entry is a file or a directory. for (const auto& i : files_) { - std::cout << "ADD FILE -------" << i << std::endl; struct stat s; stat(i.c_str(), &s); if (S_ISDIR(s.st_mode)) { @@ -64,9 +66,14 @@ class SapiLibarchiveSandboxCreate : public LibarchiveSandbox { absl::string_view archive_path_; }; +// When an archive is extracted, the generated files/directories will be placed +// relative to the current working directory. In order to add permissions to this +// we create a temporary directory at every extraction. Then, we change the directory of +// the sandboxed process to that directory and map it to the current "real" working +// directory. This way the contents of the archived will pe placed correctly without +// offering additional permission. class SapiLibarchiveSandboxExtract : public LibarchiveSandbox { public: - // TODO explicit SapiLibarchiveSandboxExtract(absl::string_view archive_path, const int do_extract, absl::string_view tmp_dir) @@ -76,6 +83,8 @@ class SapiLibarchiveSandboxExtract : public LibarchiveSandbox { private: virtual void ModifyExecutor(sandbox2::Executor* executor) override { + // If the user only wants to list the entries in the archive, we do + // not need to worry about changing directories; if (do_extract_) { executor = &executor->set_cwd(std::string(tmp_dir_)); } @@ -105,7 +114,6 @@ class SapiLibarchiveSandboxExtract : public LibarchiveSandbox { .AddFile(archive_path_); if (do_extract_) { - // map "/output/" to cwd std::string cwd = sandbox2::file_util::fileops::GetCWD(); policy = policy.AddDirectoryAt(cwd, tmp_dir_, false); } diff --git a/oss-internship-2020/sapi_libarchive/examples/sapi_minitar.cc b/oss-internship-2020/sapi_libarchive/examples/sapi_minitar.cc index 833f972..d71e277 100644 --- a/oss-internship-2020/sapi_libarchive/examples/sapi_minitar.cc +++ b/oss-internship-2020/sapi_libarchive/examples/sapi_minitar.cc @@ -39,6 +39,8 @@ * lzma/xz, etc. Just fill in the appropriate setup calls. */ + // TODO general comments here + #include #include #include @@ -226,7 +228,7 @@ int main(int argc, const char** argv) { static void create(const char* initial_filename, int compress, const char** argv) { // We split the filename path into dirname and filename. To the filename we - // prepend /output/ so that it will work with the security policy. + // prepend "/output/"" so that it will work with the security policy. std::string abs_path = MakeAbsolutePathAtCWD(std::string(initial_filename)); auto [archive_path, filename_tmp] = sandbox2::file::SplitPath(abs_path); @@ -243,6 +245,10 @@ static void create(const char* initial_filename, int compress, std::transform(relative_paths.begin(), relative_paths.end(), relative_paths.begin(), sandbox2::file::CleanPath); + // At this point, we have the cleaned relative and absolute paths saved + // in vectors. + + // Initialize sandbox and api object SapiLibarchiveSandboxCreate sandbox(absolute_paths, archive_path); CHECK(sandbox.Init().ok()) << "Error during sandbox initialization"; LibarchiveApi api(&sandbox); @@ -251,6 +257,8 @@ static void create(const char* initial_filename, int compress, CHECK(ret.ok()) << "write_new call failed"; CHECK(ret.value() != NULL) << "Failed to create write archive"; + // Treat the pointer as remote. There is no need to copy the data + // to the client process. sapi::v::RemotePtr a(ret.value()); absl::StatusOr ret2; @@ -307,6 +315,7 @@ static void create(const char* initial_filename, int compress, int file_idx = 0; + // We can directly use the vectors defined before. for (int file_idx = 0; file_idx < absolute_paths.size(); ++file_idx) { ret = api.archive_read_disk_new(); CHECK(ret.ok()) << "read_disk_new call failed"; @@ -320,7 +329,7 @@ static void create(const char* initial_filename, int compress, CHECK(ret2.value() != ARCHIVE_FATAL) << "Unexpected result from read_disk_set_standard_lookup call"; #endif - + // We use the absolute path first. ret2 = api.archive_read_disk_open( &disk, sapi::v::ConstCStr(absolute_paths[file_idx].c_str()).PtrBefore()); @@ -359,7 +368,8 @@ static void create(const char* initial_filename, int compress, // absolute path prefix with the relative one. Example: we add the folder // test_files which becomes /absolute/path/test_files and the files inside // of it will become /absolute/path/test_files/file1 and we change it to - // test_files/file1 so that it is relative. + // test_files/file1 so that it is relative. This only changes the pathname + // so that relative paths are preserved. std::string path_name = CheckStatusAndGetString(api.archive_entry_pathname(&entry), sandbox); @@ -403,6 +413,9 @@ static void create(const char* initial_filename, int compress, CHECK(ret2.value() != ARCHIVE_FATAL) << "Unexpected result from write_header call"; + // In the following section, the calls (read, archive_write_data) are done on + // the sandboxed process since we do not need to transfer the data in the client + // process. if (ret2.value() > ARCHIVE_FAILED) { int fd = open(CheckStatusAndGetString( api.archive_entry_sourcepath(&entry), sandbox) @@ -415,9 +428,11 @@ static void create(const char* initial_filename, int compress, sapi::v::Array buff(kBuffSize); sapi::v::UInt ssize(kBuffSize); + // We allocate the buffer remotely and then we can simply use the remote pointer. CHECK(sandbox.Allocate(&buff, true).ok()) << "Could not allocate remote buffer"; + // We can use sapi objects that help us with file descriptors. CHECK(sandbox.TransferToSandboxee(&sapi_fd).ok()) << "Could not transfer file descriptor"; @@ -555,6 +570,8 @@ static void extract(const char* filename, int do_extract, int flags) { filename_ptr = NULL; } + // The entries are saved with a relative path so they are all created + // relative to the current working directory. ret2 = api.archive_read_open_filename( &a, sapi::v::ConstCStr(filename_ptr).PtrBefore(), kBlockSize); CHECK(ret2.ok()) << "read_open_filename call failed"; @@ -600,7 +617,7 @@ static void extract(const char* filename, int do_extract, int flags) { needcr = 1; } } - // use the needcr stuff here TODO + if (needcr) { std::cout << std::endl; } @@ -623,6 +640,8 @@ static void extract(const char* filename, int do_extract, int flags) { CHECK(!ret2.value()) << "Unexpected result from write_free call"; } +// This function is only called from the "extract function". It is still isolated +// in order to not modify the code structure as much. static int copy_data(sapi::v::RemotePtr* ar, sapi::v::RemotePtr* aw, LibarchiveApi& api, SapiLibarchiveSandboxExtract& sandbox) { @@ -661,15 +680,6 @@ static int copy_data(sapi::v::RemotePtr* ar, sapi::v::RemotePtr* aw, } } -// static void msg(const char* m) { write(1, m, strlen(m)); } - -// static void errmsg(const char* m) { -// if (m == NULL) { -// m = "Error: No error description provided.\n"; -// } -// write(2, m, strlen(m)); -// } - static void usage(void) { /* Many program options depend on compile options. */ const char* m =