Skip to content

Commit

Permalink
policy: do not handle fd
Browse files Browse the repository at this point in the history
  • Loading branch information
@RafaelGSS
RafaelGSS committed Jul 20, 2022
1 parent 8188970 commit 246cb0f
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 76 deletions.
5 changes: 5 additions & 0 deletions src/node_dir.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -320,6 +321,10 @@ static void OpenDir(const FunctionCallbackInfo<Value>& 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);

Expand Down
51 changes: 38 additions & 13 deletions src/node_file.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,8 +866,10 @@ void Access(const FunctionCallbackInfo<Value>& 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)
Expand Down Expand Up @@ -1653,8 +1655,10 @@ static void ReadDir(const FunctionCallbackInfo<Value>& 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);

Expand Down Expand Up @@ -1741,16 +1745,28 @@ static void Open(const FunctionCallbackInfo<Value>& 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<Int32>()->Value();

CHECK(args[2]->IsInt32());
const int mode = args[2].As<Int32>()->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);
Expand Down Expand Up @@ -1787,6 +1803,21 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
CHECK(args[2]->IsInt32());
const int mode = args[2].As<Int32>()->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,
Expand Down Expand Up @@ -1856,8 +1887,6 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {

CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, fd);

CHECK(Buffer::HasInstance(args[1]));
Local<Object> buffer_obj = args[1].As<Object>();
Expand Down Expand Up @@ -1958,8 +1987,6 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
CHECK_GE(argc, 4);
CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, fd);

const int64_t pos = GetOffset(args[2]);

Expand Down Expand Up @@ -2061,8 +2088,6 @@ static void Read(const FunctionCallbackInfo<Value>& args) {

CHECK(args[0]->IsInt32());
const int fd = args[0].As<Int32>()->Value();
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemIn, fd);

CHECK(Buffer::HasInstance(args[1]));
Local<Object> buffer_obj = args[1].As<Object>();
Expand Down
10 changes: 1 addition & 9 deletions src/policy/policy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/policy/policy_deny.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ class PolicyDeny {
virtual v8::Maybe<bool> Apply(const std::string& deny) = 0;
virtual bool Deny(Permission scope, std::vector<std::string> 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
Expand Down
5 changes: 0 additions & 5 deletions src/policy/policy_deny_fs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/policy/policy_deny_fs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class PolicyDenyFs : public PolicyDeny {
Maybe<bool> Apply(const std::string& deny);
bool Deny(Permission scope, std::vector<std::string> 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);
Expand Down
73 changes: 28 additions & 45 deletions test/parallel/test-policy-deny-fs-in.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, () => {});
Expand Down Expand Up @@ -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, () => {});
Expand All @@ -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, () => {});
Expand Down

0 comments on commit 246cb0f

Please sign in to comment.