diff --git a/src/node_dir.cc b/src/node_dir.cc index c530da0cc3be4b..ab1517d4b1d2ab 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -4,6 +4,7 @@ #include "node_process-inl.h" #include "memory_tracker-inl.h" #include "util.h" +#include "policy/policy.h" #include "tracing/trace_event.h" @@ -320,6 +321,10 @@ static void OpenDir(const FunctionCallbackInfo& args) { BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); + // TODO(rafaelgss): likely it will need to be handled in the JS only + // See: https://github.com/nodejs/node/pull/35487 + /* THROW_IF_INSUFFICIENT_PERMISSIONS( */ + /* env, policy::Permission::kFileSystemIn, *path); */ const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); diff --git a/src/node_file.cc b/src/node_file.cc index 909f09d9f43929..aab987d0e6d51e 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -871,8 +871,10 @@ void Access(const FunctionCallbackInfo& args) { BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, policy::Permission::kFileSystemIn, *path); + // TODO(rafaelgss): likely it will need to be handled in the JS only + // See: https://github.com/nodejs/node/pull/35487 + /* THROW_IF_INSUFFICIENT_PERMISSIONS( */ + /* env, policy::Permission::kFileSystemIn, *path); */ FSReqBase* req_wrap_async = GetReqWrap(args, 2); if (req_wrap_async != nullptr) { // access(path, mode, req) @@ -1659,8 +1661,10 @@ static void ReadDir(const FunctionCallbackInfo& args) { BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, policy::Permission::kFileSystemIn, *path); + // TODO(rafaelgss): likely it will need to be handled in the JS only + // See: https://github.com/nodejs/node/pull/35487 + /* THROW_IF_INSUFFICIENT_PERMISSIONS( */ + /* env, policy::Permission::kFileSystemIn, *path); */ const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); @@ -1747,9 +1751,6 @@ static void Open(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); - // Open can be called either in write or read - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, policy::Permission::kFileSystem, *path); CHECK(args[1]->IsInt32()); const int flags = args[1].As()->Value(); @@ -1757,6 +1758,21 @@ static void Open(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); const int mode = args[2].As()->Value(); + // Open can be called either in write or read + if (flags == O_RDWR) { + // TODO(rafaelgss): it can be optimized to void n*m + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemIn, *path); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemOut, *path); + } else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemIn, *path); + } else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemOut, *path); + } + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // open(path, flags, mode, req) req_wrap_async->set_is_plain_open(true); @@ -1793,6 +1809,21 @@ static void OpenFileHandle(const FunctionCallbackInfo& args) { CHECK(args[2]->IsInt32()); const int mode = args[2].As()->Value(); + // Open can be called either in write or read + if (flags == O_RDWR) { + // TODO(rafaelgss): it can be optimized to void n*m + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemIn, *path); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemOut, *path); + } else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemIn, *path); + } else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, policy::Permission::kFileSystemOut, *path); + } + FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // openFileHandle(path, flags, mode, req) AsyncCall(env, req_wrap_async, args, "open", UTF8, AfterOpenFileHandle, @@ -1862,8 +1893,6 @@ static void WriteBuffer(const FunctionCallbackInfo& args) { CHECK(args[0]->IsInt32()); const int fd = args[0].As()->Value(); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, policy::Permission::kFileSystemOut, fd); CHECK(Buffer::HasInstance(args[1])); Local buffer_obj = args[1].As(); @@ -1964,8 +1993,6 @@ static void WriteString(const FunctionCallbackInfo& args) { CHECK_GE(argc, 4); CHECK(args[0]->IsInt32()); const int fd = args[0].As()->Value(); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, policy::Permission::kFileSystemOut, fd); const int64_t pos = GetOffset(args[2]); @@ -2067,8 +2094,6 @@ static void Read(const FunctionCallbackInfo& args) { CHECK(args[0]->IsInt32()); const int fd = args[0].As()->Value(); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, policy::Permission::kFileSystemIn, fd); CHECK(Buffer::HasInstance(args[1])); Local buffer_obj = args[1].As(); diff --git a/src/policy/policy.h b/src/policy/policy.h index 675bf7942d631f..27b663940f3107 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -19,7 +19,7 @@ namespace policy { #define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...) \ if (!node::policy::root_policy.is_granted(perm_, resource_)) { \ - node::policy::Policy::ThrowAccessDenied((env), perm_); \ + return node::policy::Policy::ThrowAccessDenied((env), perm_); \ } class Policy { @@ -45,14 +45,6 @@ class Policy { return is_granted(permission, res.c_str()); } - inline bool is_granted(const Permission permission, unsigned fd) { - auto policy = deny_policies.find(permission); - if (policy != deny_policies.end()) { - return policy->second->is_granted(permission, fd); - } - return false; - } - static Permission StringToPermission(std::string perm); static const char* PermissionToString(Permission perm); static void ThrowAccessDenied(Environment* env, Permission perm); diff --git a/src/policy/policy_deny.h b/src/policy/policy_deny.h index ba9f0f3f2869a7..0e1907c9fc1749 100644 --- a/src/policy/policy_deny.h +++ b/src/policy/policy_deny.h @@ -32,9 +32,6 @@ class PolicyDeny { virtual v8::Maybe Apply(const std::string& deny) = 0; virtual bool Deny(Permission scope, std::vector params) = 0; virtual bool is_granted(Permission perm, const std::string& param = "") = 0; - virtual bool is_granted(Permission perm, unsigned fd) { - return false; - }; }; } // namespace policy diff --git a/src/policy/policy_deny_fs.cc b/src/policy/policy_deny_fs.cc index 5e15d7dc5d5ef9..b320e60176a92c 100644 --- a/src/policy/policy_deny_fs.cc +++ b/src/policy/policy_deny_fs.cc @@ -117,11 +117,6 @@ bool PolicyDenyFs::is_granted(Permission perm, const std::string& param = "") { } } -bool PolicyDenyFs::is_granted(Permission perm, unsigned fd) { - // TODO(rafaelgss): FD to Filename - return true; -} - bool PolicyDenyFs::is_granted(DenyFsParams params, const std::string& opt) { char resolvedPath[PATH_MAX]; realpath(opt.c_str(), resolvedPath); diff --git a/src/policy/policy_deny_fs.h b/src/policy/policy_deny_fs.h index 402cd5642366f2..4c3d1d557d0241 100644 --- a/src/policy/policy_deny_fs.h +++ b/src/policy/policy_deny_fs.h @@ -22,7 +22,6 @@ class PolicyDenyFs : public PolicyDeny { Maybe Apply(const std::string& deny); bool Deny(Permission scope, std::vector params); bool is_granted(Permission perm, const std::string& param); - bool is_granted(Permission perm, unsigned fd); private: static bool is_granted(DenyFsParams params, const std::string& opt); void RestrictAccess(Permission scope, const std::string& param); diff --git a/test/parallel/test-policy-deny-fs-in.js b/test/parallel/test-policy-deny-fs-in.js index 743f539db79181..ff71ce064a65ff 100644 --- a/test/parallel/test-policy-deny-fs-in.js +++ b/test/parallel/test-policy-deny-fs-in.js @@ -126,42 +126,23 @@ const regularFile = __filename; }); } -// fs.accessSync -{ - assert.throws(() => { - fs.accessSync(blockedFile, fs.constants.R_OK); - }, common.expectsError({ - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemIn', - })); - - assert.throws(() => { - fs.accessSync(blockedFolder + 'anyfile', fs.constants.R_OK); - }, common.expectsError({ - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemIn', - })); - - assert.doesNotThrow(() => { - fs.accessSync(regularFile, fs.constants.R_OK); - }); -} +// TODO // fs.access { - assert.throws(() => { - fs.access(blockedFile, fs.constants.R_OK, () => {}); - }, common.expectsError({ - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemIn', - })); - - assert.throws(() => { - fs.access(blockedFolder + 'anyfile', fs.constants.R_OK, () => {}); - }, common.expectsError({ - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemIn', - })); + // assert.throws(() => { + // fs.access(blockedFile, fs.constants.R_OK, () => {}); + // }, common.expectsError({ + // code: 'ERR_ACCESS_DENIED', + // permission: 'FileSystemIn', + // })); + + // assert.throws(() => { + // fs.access(blockedFolder + 'anyfile', fs.constants.R_OK, () => {}); + // }, common.expectsError({ + // code: 'ERR_ACCESS_DENIED', + // permission: 'FileSystemIn', + // })); assert.doesNotThrow(() => { fs.access(regularFile, fs.constants.R_OK, () => {}); @@ -240,12 +221,14 @@ const regularFile = __filename; // fs.opendir (TODO) { - assert.throws(() => { - fs.opendir(blockedFolder, () => {}); - }, common.expectsError({ - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemIn', - })); + // assert.throws(() => { + // fs.opendir(blockedFolder, (err) => { + // if (err) throw err; + // }); + // }, common.expectsError({ + // code: 'ERR_ACCESS_DENIED', + // permission: 'FileSystemIn', + // })); assert.doesNotThrow(() => { fs.opendir(__dirname, () => {}); @@ -254,12 +237,12 @@ const regularFile = __filename; // fs.readdir { - assert.throws(() => { - fs.readdir(blockedFolder, () => {}); - }, common.expectsError({ - code: 'ERR_ACCESS_DENIED', - permission: 'FileSystemIn', - })); + // assert.throws(() => { + // fs.readdir(blockedFolder, () => {}); + // }, common.expectsError({ + // code: 'ERR_ACCESS_DENIED', + // permission: 'FileSystemIn', + // })); assert.doesNotThrow(() => { fs.readdir(__dirname, () => {});