Skip to content

Commit

Permalink
permission: drop process.permission.deny
Browse files Browse the repository at this point in the history
  • Loading branch information
RafaelGSS committed Mar 31, 2023
1 parent 0094f90 commit f779526
Show file tree
Hide file tree
Showing 33 changed files with 415 additions and 716 deletions.
19 changes: 0 additions & 19 deletions benchmark/permission/permission-fs-deny.js

This file was deleted.

46 changes: 3 additions & 43 deletions doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -492,24 +492,7 @@ using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.

When enabling the Permission Model through the [`--experimental-permission`][]
flag a new property `permission` is added to the `process` object.
This property contains two functions:

##### `permission.deny(scope [,parameters])`

API call to deny permissions at runtime ([`permission.deny()`][])

```js
process.permission.deny('fs'); // Deny permissions to ALL fs operations

// Deny permissions to ALL FileSystemWrite operations
process.permission.deny('fs.write');
// deny FileSystemWrite permissions to the protected-folder
process.permission.deny('fs.write', ['/home/rafaelgss/protected-folder']);
// Deny permissions to ALL FileSystemRead operations
process.permission.deny('fs.read');
// deny FileSystemRead permissions to the protected-folder
process.permission.deny('fs.read', ['/home/rafaelgss/protected-folder']);
```
This property contains one function:

##### `permission.has(scope ,parameters)`

Expand All @@ -519,10 +502,8 @@ API call to check permissions at runtime ([`permission.has()`][])
process.permission.has('fs.write'); // true
process.permission.has('fs.write', '/home/rafaelgss/protected-folder'); // true

process.permission.deny('fs.write', '/home/rafaelgss/protected-folder');

process.permission.has('fs.write'); // true
process.permission.has('fs.write', '/home/rafaelgss/protected-folder'); // false
process.permission.has('fs.read'); // true
process.permission.has('fs.read', '/home/rafaelgss/protected-folder'); // false
```

#### File System Permissions
Expand Down Expand Up @@ -560,39 +541,18 @@ There are constraints you need to know before using this system:

* Native modules are restricted by default when using the Permission Model.
* Relative paths are not supported through the CLI (`--allow-fs-*`).
The runtime API supports relative paths.
* The model does not inherit to a child node process.
* The model does not inherit to a worker thread.
* When creating symlinks the target (first argument) should have read and
write access.
* Permission changes are not retroactively applied to existing resources.
Consider the following snippet:
```js
const fs = require('node:fs');

// Open a fd
const fd = fs.openSync('./README.md', 'r');
// Then, deny access to all fs.read operations
process.permission.deny('fs.read');
// This call will NOT fail and the file will be read
const data = fs.readFileSync(fd);
```

Therefore, when possible, apply the permissions rules before any statement:

```js
process.permission.deny('fs.read');
const fd = fs.openSync('./README.md', 'r');
// Error: Access to this API has been restricted
```

[Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
[`--allow-child-process`]: cli.md#--allow-child-process
[`--allow-fs-read`]: cli.md#--allow-fs-read
[`--allow-fs-write`]: cli.md#--allow-fs-write
[`--allow-worker`]: cli.md#--allow-worker
[`--experimental-permission`]: cli.md#--experimental-permission
[`permission.deny()`]: process.md#processpermissiondenyscope-reference
[`permission.has()`]: process.md#processpermissionhasscope-reference
[import maps]: https://url.spec.whatwg.org/#relative-url-with-fragment-string
[relative-url string]: https://url.spec.whatwg.org/#relative-url-with-fragment-string
Expand Down
28 changes: 0 additions & 28 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -2634,34 +2634,6 @@ This API is available through the [`--experimental-permission`][] flag.
for the current process. Additional documentation is available in the
[Permission Model][].
### `process.permission.deny(scope[, reference])`
<!-- YAML
added: REPLACEME
-->
* `scopes` {string}
* `reference` {Array}
* Returns: {boolean}
Deny permissions at runtime.
The available scopes are:
* `fs` - All File System
* `fs.read` - File System read operations
* `fs.write` - File System write operations
The reference has a meaning based on the provided scope. For example,
the reference when the scope is File System means files and folders.
```js
// Deny READ operations to the ./README.md file
process.permission.deny('fs.read', ['./README.md']);
// Deny ALL WRITE operations
process.permission.deny('fs.write');
```
### `process.permission.has(scope[, reference])`
<!-- YAML
Expand Down
4 changes: 2 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -784,10 +784,10 @@ Environment::Environment(IsolateData* isolate_data,
if (!options_->allow_fs_read.empty() || !options_->allow_fs_write.empty()) {
options_->allow_native_addons = false;
if (!options_->allow_child_process) {
permission()->Deny(permission::PermissionScope::kChildProcess, {});
permission()->Apply("*", permission::PermissionScope::kChildProcess);
}
if (!options_->allow_worker_threads) {
permission()->Deny(permission::PermissionScope::kWorkerThreads, {});
permission()->Apply("*", permission::PermissionScope::kWorkerThreads);
}
}

Expand Down
6 changes: 1 addition & 5 deletions src/permission/child_process_permission.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ namespace permission {
// Currently, ChildProcess manage a single state
// Once denied, it's always denied
void ChildProcessPermission::Apply(const std::string& deny,
PermissionScope scope) {}

bool ChildProcessPermission::Deny(PermissionScope perm,
const std::vector<std::string>& params) {
PermissionScope scope) {
deny_all_ = true;
return true;
}

bool ChildProcessPermission::is_granted(PermissionScope perm,
Expand Down
2 changes: 0 additions & 2 deletions src/permission/child_process_permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ namespace permission {
class ChildProcessPermission final : public PermissionBase {
public:
void Apply(const std::string& deny, PermissionScope scope) override;
bool Deny(PermissionScope scope,
const std::vector<std::string>& params) override;
bool is_granted(PermissionScope perm,
const std::string_view& param = "") override;

Expand Down
46 changes: 5 additions & 41 deletions src/permission/fs_permission.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ void FreeRecursivelyNode(
delete node;
}

bool is_tree_granted(node::permission::FSPermission::RadixTree* deny_tree,
node::permission::FSPermission::RadixTree* granted_tree,
bool is_tree_granted(node::permission::FSPermission::RadixTree* granted_tree,
const std::string_view& param) {
#ifdef _WIN32
// is UNC file path
Expand All @@ -60,11 +59,10 @@ bool is_tree_granted(node::permission::FSPermission::RadixTree* deny_tree,
starting_pos += 4; // "UNC\"
}
auto normalized = param.substr(starting_pos);
return !deny_tree->Lookup(normalized) &&
granted_tree->Lookup(normalized, true);
return granted_tree->Lookup(normalized, true);
}
#endif
return !deny_tree->Lookup(param) && granted_tree->Lookup(param, true);
return granted_tree->Lookup(param, true);
}

} // namespace
Expand All @@ -91,40 +89,6 @@ void FSPermission::Apply(const std::string& allow, PermissionScope scope) {
}
}

bool FSPermission::Deny(PermissionScope perm,
const std::vector<std::string>& params) {
if (perm == PermissionScope::kFileSystem) {
deny_all_in_ = true;
deny_all_out_ = true;
return true;
}

bool deny_all = params.size() == 0;
if (perm == PermissionScope::kFileSystemRead) {
if (deny_all) deny_all_in_ = true;
// when deny_all_in is already true permission.deny should be idempotent
if (deny_all_in_) return true;
allow_all_in_ = false;
for (auto& param : params) {
deny_in_fs_.Insert(WildcardIfDir(param));
}
return true;
}

if (perm == PermissionScope::kFileSystemWrite) {
if (deny_all) deny_all_out_ = true;
// when deny_all_out is already true permission.deny should be idempotent
if (deny_all_out_) return true;
allow_all_out_ = false;

for (auto& param : params) {
deny_out_fs_.Insert(WildcardIfDir(param));
}
return true;
}
return false;
}

void FSPermission::GrantAccess(PermissionScope perm, std::string res) {
const std::string path = WildcardIfDir(res);
if (perm == PermissionScope::kFileSystemRead) {
Expand All @@ -144,11 +108,11 @@ bool FSPermission::is_granted(PermissionScope perm,
case PermissionScope::kFileSystemRead:
return !deny_all_in_ &&
((param.empty() && allow_all_in_) || allow_all_in_ ||
is_tree_granted(&deny_in_fs_, &granted_in_fs_, param));
is_tree_granted(&granted_in_fs_, param));
case PermissionScope::kFileSystemWrite:
return !deny_all_out_ &&
((param.empty() && allow_all_out_) || allow_all_out_ ||
is_tree_granted(&deny_out_fs_, &granted_out_fs_, param));
is_tree_granted(&granted_out_fs_, param));
default:
return false;
}
Expand Down
11 changes: 0 additions & 11 deletions src/permission/fs_permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ namespace permission {
class FSPermission final : public PermissionBase {
public:
void Apply(const std::string& deny, PermissionScope scope) override;
bool Deny(PermissionScope scope,
const std::vector<std::string>& params) override;
bool is_granted(PermissionScope perm, const std::string_view& param) override;

// For debugging purposes, use the gist function to print the whole tree
Expand Down Expand Up @@ -135,18 +133,9 @@ class FSPermission final : public PermissionBase {
void GrantAccess(PermissionScope scope, std::string param);
void RestrictAccess(PermissionScope scope,
const std::vector<std::string>& params);
// /tmp/* --grant
// /tmp/dsadsa/t.js denied in runtime
//
// /tmp/text.txt -- grant
// /tmp/text.txt -- denied in runtime
//
// fs granted on startup
RadixTree granted_in_fs_;
RadixTree granted_out_fs_;
// fs denied in runtime
RadixTree deny_in_fs_;
RadixTree deny_out_fs_;

bool deny_all_in_ = true;
bool deny_all_out_ = true;
Expand Down
49 changes: 0 additions & 49 deletions src/permission/permission.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@

namespace node {

using v8::Array;
using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Integer;
using v8::Local;
using v8::Object;
using v8::String;
Expand All @@ -27,42 +25,6 @@ namespace permission {

namespace {

// permission.deny('fs.read', ['/tmp/'])
// permission.deny('fs.read')
static void Deny(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
v8::Isolate* isolate = env->isolate();
CHECK(args[0]->IsString());
std::string deny_scope = *String::Utf8Value(isolate, args[0]);
PermissionScope scope = Permission::StringToPermission(deny_scope);
if (scope == PermissionScope::kPermissionsRoot) {
return args.GetReturnValue().Set(false);
}

std::vector<std::string> params;
if (args.Length() == 1 || args[1]->IsUndefined()) {
return args.GetReturnValue().Set(env->permission()->Deny(scope, params));
}

CHECK(args[1]->IsArray());
Local<Array> js_params = Local<Array>::Cast(args[1]);
Local<Context> context = isolate->GetCurrentContext();

for (uint32_t i = 0; i < js_params->Length(); ++i) {
Local<Value> arg;
if (!js_params->Get(context, Integer::New(isolate, i)).ToLocal(&arg)) {
return;
}
String::Utf8Value utf8_arg(isolate, arg);
if (*utf8_arg == nullptr) {
return;
}
params.push_back(*utf8_arg);
}

return args.GetReturnValue().Set(env->permission()->Deny(scope, params));
}

// permission.has('fs.in', '/tmp/')
// permission.has('fs.in')
static void Has(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -169,27 +131,16 @@ void Permission::Apply(const std::string& allow, PermissionScope scope) {
}
}

bool Permission::Deny(PermissionScope scope,
const std::vector<std::string>& params) {
auto permission = nodes_.find(scope);
if (permission != nodes_.end()) {
return permission->second->Deny(scope, params);
}
return false;
}

void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
SetMethod(context, target, "deny", Deny);
SetMethodNoSideEffect(context, target, "has", Has);

target->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen).FromJust();
}

void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(Deny);
registry->Register(Has);
}

Expand Down
2 changes: 0 additions & 2 deletions src/permission/permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ class Permission {

// CLI Call
void Apply(const std::string& deny, PermissionScope scope);
// Permission.Deny API
bool Deny(PermissionScope scope, const std::vector<std::string>& params);
void EnablePermissions();

private:
Expand Down
2 changes: 0 additions & 2 deletions src/permission/permission_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ enum class PermissionScope {
class PermissionBase {
public:
virtual void Apply(const std::string& deny, PermissionScope scope) = 0;
virtual bool Deny(PermissionScope scope,
const std::vector<std::string>& params) = 0;
virtual bool is_granted(PermissionScope perm,
const std::string_view& param = "") = 0;
};
Expand Down
6 changes: 1 addition & 5 deletions src/permission/worker_permission.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,8 @@ namespace permission {

// Currently, PolicyDenyWorker manage a single state
// Once denied, it's always denied
void WorkerPermission::Apply(const std::string& deny, PermissionScope scope) {}

bool WorkerPermission::Deny(PermissionScope perm,
const std::vector<std::string>& params) {
void WorkerPermission::Apply(const std::string& deny, PermissionScope scope) {
deny_all_ = true;
return true;
}

bool WorkerPermission::is_granted(PermissionScope perm,
Expand Down
2 changes: 0 additions & 2 deletions src/permission/worker_permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ namespace permission {
class WorkerPermission final : public PermissionBase {
public:
void Apply(const std::string& deny, PermissionScope scope) override;
bool Deny(PermissionScope scope,
const std::vector<std::string>& params) override;
bool is_granted(PermissionScope perm,
const std::string_view& param = "") override;

Expand Down
Loading

0 comments on commit f779526

Please sign in to comment.