Skip to content

Commit 246cb0f

Browse files
RafaelGSSRafaelGSS
committed
policy: do not handle fd
1 parent 8188970 commit 246cb0f

File tree

7 files changed

+72
-76
lines changed

7 files changed

+72
-76
lines changed

‎src/node_dir.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "node_process-inl.h"
55
#include "memory_tracker-inl.h"
66
#include "util.h"
7+
#include "policy/policy.h"
78

89
#include "tracing/trace_event.h"
910

@@ -320,6 +321,10 @@ static void OpenDir(const FunctionCallbackInfo<Value>& args) {
320321

321322
BufferValue path(isolate, args[0]);
322323
CHECK_NOT_NULL(*path);
324+
// TODO(rafaelgss): likely it will need to be handled in the JS only
325+
// See: https://github.com/nodejs/node/pull/35487
326+
/* THROW_IF_INSUFFICIENT_PERMISSIONS( */
327+
/* env, policy::Permission::kFileSystemIn, *path); */
323328

324329
const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);
325330

‎src/node_file.cc‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -866,8 +866,10 @@ void Access(const FunctionCallbackInfo<Value>& args) {
866866

867867
BufferValue path(isolate, args[0]);
868868
CHECK_NOT_NULL(*path);
869-
THROW_IF_INSUFFICIENT_PERMISSIONS(
870-
env, policy::Permission::kFileSystemIn, *path);
869+
// TODO(rafaelgss): likely it will need to be handled in the JS only
870+
// See: https://github.com/nodejs/node/pull/35487
871+
/* THROW_IF_INSUFFICIENT_PERMISSIONS( */
872+
/* env, policy::Permission::kFileSystemIn, *path); */
871873

872874
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
873875
if (req_wrap_async != nullptr) { // access(path, mode, req)
@@ -1653,8 +1655,10 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
16531655

16541656
BufferValue path(isolate, args[0]);
16551657
CHECK_NOT_NULL(*path);
1656-
THROW_IF_INSUFFICIENT_PERMISSIONS(
1657-
env, policy::Permission::kFileSystemIn, *path);
1658+
// TODO(rafaelgss): likely it will need to be handled in the JS only
1659+
// See: https://github.com/nodejs/node/pull/35487
1660+
/* THROW_IF_INSUFFICIENT_PERMISSIONS( */
1661+
/* env, policy::Permission::kFileSystemIn, *path); */
16581662

16591663
const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);
16601664

@@ -1741,16 +1745,28 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
17411745

17421746
BufferValue path(env->isolate(), args[0]);
17431747
CHECK_NOT_NULL(*path);
1744-
// Open can be called either in write or read
1745-
THROW_IF_INSUFFICIENT_PERMISSIONS(
1746-
env, policy::Permission::kFileSystem, *path);
17471748

17481749
CHECK(args[1]->IsInt32());
17491750
const int flags = args[1].As<Int32>()->Value();
17501751

17511752
CHECK(args[2]->IsInt32());
17521753
const int mode = args[2].As<Int32>()->Value();
17531754

1755+
// Open can be called either in write or read
1756+
if (flags == O_RDWR) {
1757+
// TODO(rafaelgss): it can be optimized to void n*m
1758+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1759+
env, policy::Permission::kFileSystemIn, *path);
1760+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1761+
env, policy::Permission::kFileSystemOut, *path);
1762+
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
1763+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1764+
env, policy::Permission::kFileSystemIn, *path);
1765+
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
1766+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1767+
env, policy::Permission::kFileSystemOut, *path);
1768+
}
1769+
17541770
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
17551771
if (req_wrap_async != nullptr) { // open(path, flags, mode, req)
17561772
req_wrap_async->set_is_plain_open(true);
@@ -1787,6 +1803,21 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
17871803
CHECK(args[2]->IsInt32());
17881804
const int mode = args[2].As<Int32>()->Value();
17891805

1806+
// Open can be called either in write or read
1807+
if (flags == O_RDWR) {
1808+
// TODO(rafaelgss): it can be optimized to void n*m
1809+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1810+
env, policy::Permission::kFileSystemIn, *path);
1811+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1812+
env, policy::Permission::kFileSystemOut, *path);
1813+
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
1814+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1815+
env, policy::Permission::kFileSystemIn, *path);
1816+
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
1817+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1818+
env, policy::Permission::kFileSystemOut, *path);
1819+
}
1820+
17901821
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
17911822
if (req_wrap_async != nullptr) { // openFileHandle(path, flags, mode, req)
17921823
AsyncCall(env, req_wrap_async, args, "open", UTF8, AfterOpenFileHandle,
@@ -1856,8 +1887,6 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
18561887

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

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

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

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

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

20672092
CHECK(Buffer::HasInstance(args[1]));
20682093
Local<Object> buffer_obj = args[1].As<Object>();

‎src/policy/policy.h‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace policy {
1919

2020
#define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...) \
2121
if (!node::policy::root_policy.is_granted(perm_, resource_)) { \
22-
node::policy::Policy::ThrowAccessDenied((env), perm_); \
22+
return node::policy::Policy::ThrowAccessDenied((env), perm_); \
2323
}
2424

2525
class Policy {
@@ -45,14 +45,6 @@ class Policy {
4545
return is_granted(permission, res.c_str());
4646
}
4747

48-
inline bool is_granted(const Permission permission, unsigned fd) {
49-
auto policy = deny_policies.find(permission);
50-
if (policy != deny_policies.end()) {
51-
return policy->second->is_granted(permission, fd);
52-
}
53-
return false;
54-
}
55-
5648
static Permission StringToPermission(std::string perm);
5749
static const char* PermissionToString(Permission perm);
5850
static void ThrowAccessDenied(Environment* env, Permission perm);

‎src/policy/policy_deny.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ class PolicyDeny {
3232
virtual v8::Maybe<bool> Apply(const std::string& deny) = 0;
3333
virtual bool Deny(Permission scope, std::vector<std::string> params) = 0;
3434
virtual bool is_granted(Permission perm, const std::string& param = "") = 0;
35-
virtual bool is_granted(Permission perm, unsigned fd) {
36-
return false;
37-
};
3835
};
3936

4037
} // namespace policy

‎src/policy/policy_deny_fs.cc‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,6 @@ bool PolicyDenyFs::is_granted(Permission perm, const std::string& param = "") {
117117
}
118118
}
119119

120-
bool PolicyDenyFs::is_granted(Permission perm, unsigned fd) {
121-
// TODO(rafaelgss): FD to Filename
122-
return true;
123-
}
124-
125120
bool PolicyDenyFs::is_granted(DenyFsParams params, const std::string& opt) {
126121
char resolvedPath[PATH_MAX];
127122
realpath(opt.c_str(), resolvedPath);

‎src/policy/policy_deny_fs.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ class PolicyDenyFs : public PolicyDeny {
2222
Maybe<bool> Apply(const std::string& deny);
2323
bool Deny(Permission scope, std::vector<std::string> params);
2424
bool is_granted(Permission perm, const std::string& param);
25-
bool is_granted(Permission perm, unsigned fd);
2625
private:
2726
static bool is_granted(DenyFsParams params, const std::string& opt);
2827
void RestrictAccess(Permission scope, const std::string& param);

‎test/parallel/test-policy-deny-fs-in.js‎

Lines changed: 28 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -126,42 +126,23 @@ const regularFile = __filename;
126126
});
127127
}
128128

129-
// fs.accessSync
130-
{
131-
assert.throws(() => {
132-
fs.accessSync(blockedFile, fs.constants.R_OK);
133-
}, common.expectsError({
134-
code: 'ERR_ACCESS_DENIED',
135-
permission: 'FileSystemIn',
136-
}));
137-
138-
assert.throws(() => {
139-
fs.accessSync(blockedFolder + 'anyfile', fs.constants.R_OK);
140-
}, common.expectsError({
141-
code: 'ERR_ACCESS_DENIED',
142-
permission: 'FileSystemIn',
143-
}));
144-
145-
assert.doesNotThrow(() => {
146-
fs.accessSync(regularFile, fs.constants.R_OK);
147-
});
148-
}
149129

130+
// TODO
150131
// fs.access
151132
{
152-
assert.throws(() => {
153-
fs.access(blockedFile, fs.constants.R_OK, () => {});
154-
}, common.expectsError({
155-
code: 'ERR_ACCESS_DENIED',
156-
permission: 'FileSystemIn',
157-
}));
158-
159-
assert.throws(() => {
160-
fs.access(blockedFolder + 'anyfile', fs.constants.R_OK, () => {});
161-
}, common.expectsError({
162-
code: 'ERR_ACCESS_DENIED',
163-
permission: 'FileSystemIn',
164-
}));
133+
// assert.throws(() => {
134+
// fs.access(blockedFile, fs.constants.R_OK, () => {});
135+
// }, common.expectsError({
136+
// code: 'ERR_ACCESS_DENIED',
137+
// permission: 'FileSystemIn',
138+
// }));
139+
140+
// assert.throws(() => {
141+
// fs.access(blockedFolder + 'anyfile', fs.constants.R_OK, () => {});
142+
// }, common.expectsError({
143+
// code: 'ERR_ACCESS_DENIED',
144+
// permission: 'FileSystemIn',
145+
// }));
165146

166147
assert.doesNotThrow(() => {
167148
fs.access(regularFile, fs.constants.R_OK, () => {});
@@ -240,12 +221,14 @@ const regularFile = __filename;
240221

241222
// fs.opendir (TODO)
242223
{
243-
assert.throws(() => {
244-
fs.opendir(blockedFolder, () => {});
245-
}, common.expectsError({
246-
code: 'ERR_ACCESS_DENIED',
247-
permission: 'FileSystemIn',
248-
}));
224+
// assert.throws(() => {
225+
// fs.opendir(blockedFolder, (err) => {
226+
// if (err) throw err;
227+
// });
228+
// }, common.expectsError({
229+
// code: 'ERR_ACCESS_DENIED',
230+
// permission: 'FileSystemIn',
231+
// }));
249232

250233
assert.doesNotThrow(() => {
251234
fs.opendir(__dirname, () => {});
@@ -254,12 +237,12 @@ const regularFile = __filename;
254237

255238
// fs.readdir
256239
{
257-
assert.throws(() => {
258-
fs.readdir(blockedFolder, () => {});
259-
}, common.expectsError({
260-
code: 'ERR_ACCESS_DENIED',
261-
permission: 'FileSystemIn',
262-
}));
240+
// assert.throws(() => {
241+
// fs.readdir(blockedFolder, () => {});
242+
// }, common.expectsError({
243+
// code: 'ERR_ACCESS_DENIED',
244+
// permission: 'FileSystemIn',
245+
// }));
263246

264247
assert.doesNotThrow(() => {
265248
fs.readdir(__dirname, () => {});

0 commit comments

Comments
 (0)